ClickHouse / ClickHouse/clickhouse-go

Improvement: Make server-side binding for query params default

Open
#1,729 0 comments 1 reaction 0 assignees View on GitHub
improvement
Dominant language
Go
Stars
3.3k
Forks
684
Avg merge
2d 3h
Merged PRs (30d)
14

Description

This is to track the issues/experiments [discussed on this issue.](https://github.com/ClickHouse/clickhouse-go/issues/1713)

particularly [this comment.](https://github.com/ClickHouse/clickhouse-go/issues/1713#issuecomment-3585069044)

### Context

It's generally nice to handover query param binding in the query to the server side for multiple reasons.
1. No special escape handling on client-side (if error prone, can lead to SQL inject attacks)
2. Server can do certain optimizations on it's side
3. Less work for clients in general

This is common on even popular database drivers in Go ecosystem. [Say in mysql and postgres world where server side binding is default.](https://github.com/ClickHouse/clickhouse-go/issues/1713#issuecomment-3562350555)

### Experiment.

(copied from [this comment](https://github.com/ClickHouse/clickhouse-go/issues/1713#issuecomment-3585069044))

### Problem

The problem is two folds
1. ClickHouse server accepts params via `SET param_value=10; SELECT * FROM test WHERE value={value:Int64}`

NOTE: Here, the client need to **send both the value and it's type to the server.**

2. Go client currently accepts three different ways to bind the parameters.

a. Positional binding.
```go
conn.Query("SELECT * FROM test WHERE name=? AND value=?", "some-name", some-value)
```
b. Numeric binding.
```go
conn.Query("SELECT * FROM test WHERE name=$2 AND value=$1", "some-name", some-value)
```
c. ClickHouse native binding
```go
params := clickhouse.WithParameters(clickhouse.Parameters{
"value": "10",
})

ctx := clickhouse.Context(context.Background(), params)

rows, err := conn.QueryContext(ctx, "SELECT * FROM test WHERE value={value: Int64}")
```

(c) is server side binding. The problem is with (a) and (b) which are client side binded now. Notice that arguments passed for both positional binding and numeric binding are of type `any`. **Meaning we cannot infer exact type** (int vs int32 vs int64 vs float32 vs float64, etc). Basically in order to convert those into server side binding, we need to know their exact type to send it to ClickHouse server. Which we cannot.

(a) and (b) is the standard way Go's `sql.DB` works unfortunately.

(@mshustov probably this is the one you were saying that Go's standard is the blocker to migrate to server-side binding)

### Proposal
1. We can deprecate using both positional and numeric binding in our Go client and warn the user to switch to clickhouse native binding
2. Properly support both client-side and server-side binding.

Both has tradeoffs. Neither is is the single best way to solve the problem. I'm bit concerned with (1) because don't know how users (both internal and OSS) would consider this change, given this is the standard way how people using SQL in Go.

Open for any thoughts and ideas

### Current consensus

[As discussed here.](https://github.com/ClickHouse/clickhouse-go/issues/1713#issuecomment-3606092163)

I'm also tying to change the api a little bit so that it is possible to move to server-side binding even for positional and numberic bindings. [But still WIP.](https://github.com/ClickHouse/clickhouse-go/pull/1728)

Contributor guide

Open the contributing guide

Research direction

Start by reading issue #1713 and the linked pull request #1728, which discuss the server-side binding experiment and proposed API changes. Determine how positional, numeric, and ClickHouse-native bindings should coexist; done requires an agreed implementation path for making server-side binding the default without losing required compatibility.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
backend-api-design, database
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.