Skip to content

feat: support server-side parameter binding#762

Open
cliftonc wants to merge 2 commits intodatabendlabs:mainfrom
cliftonc:feat/server-side-param-binding
Open

feat: support server-side parameter binding#762
cliftonc wants to merge 2 commits intodatabendlabs:mainfrom
cliftonc:feat/server-side-param-binding

Conversation

@cliftonc
Copy link
Copy Markdown

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

When the server supports it (version > 1.2.900), send raw SQL with a JSON params field in /v1/query instead of interpolating parameters client-side. Falls back to client-side interpolation for older servers or when SQL contains $N column position placeholders (which the server uses for stage column refs).

  • Add params field to QueryRequest
  • Add server_side_params capability flag with version threshold
  • Thread params through start_query / query_all in core client
  • Add Params::to_json_value() with sql_string_to_json() reverse parser
  • Add PlaceholderVisitor::has_dollar_positions() for $N detection
  • Add *_with_params() methods to IConnection trait with defaults
  • Override in RestAPIConnection to pass params to server
  • Route in QueryBuilder/ExecBuilder: server-side when supported and no $N, client-side otherwise
  • Add to_json_params() helper in Python bindings for future use

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

ref databendlabs#759

When the server supports it (version > 1.2.900), send raw SQL with a
JSON `params` field instead of interpolating parameters client-side.
Falls back to client-side interpolation for older servers or when SQL
contains `$N` column position placeholders (which the server uses for
stage column refs).

Changes:
- Add `params` field to `QueryRequest` (core)
- Add `server_side_params` capability flag with version threshold
- Thread params through `start_query` / `query_all` in core client
- Add `Params::to_json_value()` with `sql_string_to_json()` reverse parser
- Add `PlaceholderVisitor::has_dollar_positions()` for `$N` detection
- Add `*_with_params()` methods to `IConnection` trait with defaults
- Override in `RestAPIConnection` to pass params to server
- Route in `QueryBuilder`/`ExecBuilder`: server-side when supported and
  no `$N`, client-side otherwise
- Add `to_json_params()` helper in Python bindings for future use
@cliftonc cliftonc force-pushed the feat/server-side-param-binding branch from b158eb7 to eeb1d59 Compare March 27, 2026 04:33
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b158eb790f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@sundy-li sundy-li requested a review from youngsofun March 27, 2026 04:44
- Accept case-insensitive TRUE/FALSE (serde_json::Value::Bool produces
  lowercase "true"/"false")
- Try u64 parse before f64 to avoid precision loss on values above
  i64::MAX
- Only attempt f64 parse when string contains '.', 'e', or 'E'
- Add test cases for both edge cases
if s.eq_ignore_ascii_case("TRUE") {
return serde_json::Value::Bool(true);
}
if s.eq_ignore_ascii_case("FALSE") {
Copy link
Copy Markdown
Member

@youngsofun youngsofun Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be better to avoid this Reverse-parse?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants