Conversation
Add a public sf_core::bindings module that constructs Snowflake
parameter-binding JSON from typed Rust values via a ParamValue enum and
two builders (to_json_single for one row, to_json_arrays for array
binding). The wire-text encodings match the ODBC converters in
odbc/src/conversion byte-for-byte, so a query bound through either path
produces the same server-side value.
Until now sf_core accepted only pre-serialized JSON binding bytes
(BindingType::Json points at raw UTF-8), leaving each caller and test to
hand-write the {"1": {"type", "value"}} payload. The new builder gives
Rust callers a typed, reusable entry point and centralizes the encoding
rules: every value is emitted as a JSON string, TIMESTAMP_LTZ is tagged
TEXT, TIMESTAMP_TZ applies the +1440 offset bias, and a fully-null column
is tagged ANY. Year-month and day-time intervals are accepted as
pre-formatted literals and documented on their enum variants.
Covered by 43 unit tests spanning every logical type, NULL handling,
array binding, string escaping, and the mixed-type and mismatched-length
error paths. cargo build, test, clippy, and fmt are clean for the crate.
Address a code-review finding on the new sf_core::bindings builder. ParamValue::TimestampTz exposes offset_minutes as an arbitrary i32, and encode_tz computed offset_minutes + 1440 unchecked, which panics on overflow in debug builds and wraps into a bogus wire offset in release. Validate offset_minutes against the driver's legal +/-1439-minute range (matching the ODBC converter) before applying the bias, returning the new BindingError::TimestampTzOffsetOutOfRange for anything wider. The range check makes the biased sum provably overflow-safe. Adds regression tests for the i32::MAX rejection and the +/-1439 boundaries.
Address a second code-review finding on sf_core::bindings. encode_wallclock
zero-padded the absolute year and prepended a sign, so year -1 rendered as
"-0001". The ODBC put_year formatter it mirrors counts the sign within the
minimum width of four, producing "-001", so TIMESTAMP_LTZ bind text diverged
for proleptic negative years.
Format the signed year with format!("{:04}", year), which counts the sign in
the width and matches the ODBC output for every year while leaving positive
years unchanged. Adds a negative-year regression test.
snowflake-copybara Bot
pushed a commit
that referenced
this pull request
Sep 3, 2026
## Summary Public `release/python-*` pushes never build wheels. Driver CI reaches the Python wheel jobs only through `detect-changes`, and that workflow uses `dorny/paths-filter` pinned to a commit SHA. The snowflakedb action allowlist matches tags, not SHAs, so GitHub rejects the whole graph at startup: > The action `dorny/paths-filter@ceb8a2b8…` is not allowed in > `snowflakedb/drivers` because all actions must be from a repository owned by > your enterprise, created by GitHub, verified in the GitHub Marketplace, or > match one of the patterns: … Every rc2 push on [release/python-v5.0.0rc2](https://github.com/snowflakedb/drivers/commits/release/python-v5.0.0rc2) ends as `startup_failure` in 0s — for example [run 33651620637](https://github.com/snowflakedb/drivers/actions/runs/33651620637). rc1 was fine because it still ran standalone Python CI on `push` to `release/python-**` with `dorny/paths-filter@v3`, which the allowlist accepts. The SHA pin landed in #1335 on Aug 21. ODBC rc2 keeps shipping through exactly this decoupling: its Driver CI run fails the same way, but `build-odbc-packages.yml` is a top-level workflow on `push: release/odbc-**` that never loads `detect-changes`, so the packages still build. JDBC has the same arrangement. `build-python-release.yml` is the Python equivalent and uses no third-party action — it was disabled when Driver CI took over, which is fine on snowflake-eng and leaves the mirror with nothing. So this just turns the push trigger back on: - `push` to `release/python-**`, gated to `snowflakedb`, since a release push on snowflake-eng already gets this matrix from Driver CI at nightly scope - `workflow_dispatch` still works on both repos - `concurrency` per ref, because Copybara lands several commits on a release branch within minutes (rc2 took three pushes in nine minutes today) and each run here is ~76 jobs The wheel targets and the cloud test matrix are untouched. The mirror does have cloud credentials — rc1's [run 32252360737](https://github.com/snowflakedb/drivers/actions/runs/32252360737) was 76 jobs on snowflakedb with aws/gcp/azure legs all green — so the test jobs work there as-is. Unpinning `dorny/paths-filter` in `detect-changes.yml` is worth doing separately, but packaging should not depend on change detection either way: on a `release/python-*` push, `plan` already enables the Python suite from the branch name, so detection contributes nothing. ## Follow-up This does not help `release/python-v5.0.0rc2` until the workflow file exists **on that branch** — GitHub reads workflows from the pushed commit. Once this merges, the release branch needs it cherry-picked (or Copybara needs to carry it) before the next push builds wheels. ## Test plan - Diff is trigger-only: the `on:` block, one `if:` on `build_wheels`, and a `concurrency` block. No job, matrix, or step body changes. - The push gate cannot run on this PR (snowflake-eng, and `pull_request` is not a trigger here), so verification is `workflow_dispatch` on the branch, which the gate's `workflow_dispatch` arm allows on either repo. ## Changelog - No user-visible change; CI packaging only. Co-authored-by: Cursor <cursoragent@cursor.com> GitOrigin-RevId: f59479f
snowflake-copybara Bot
pushed a commit
that referenced
this pull request
Sep 3, 2026
## Summary Public `release/python-*` pushes never build wheels. Driver CI reaches the Python wheel jobs only through `detect-changes`, and that workflow uses `dorny/paths-filter` pinned to a commit SHA. The snowflakedb action allowlist matches tags, not SHAs, so GitHub rejects the whole graph at startup: > The action `dorny/paths-filter@ceb8a2b8…` is not allowed in > `snowflakedb/drivers` because all actions must be from a repository owned by > your enterprise, created by GitHub, verified in the GitHub Marketplace, or > match one of the patterns: … Every rc2 push on [release/python-v5.0.0rc2](https://github.com/snowflakedb/drivers/commits/release/python-v5.0.0rc2) ends as `startup_failure` in 0s — for example [run 33651620637](https://github.com/snowflakedb/drivers/actions/runs/33651620637). rc1 was fine because it still ran standalone Python CI on `push` to `release/python-**` with `dorny/paths-filter@v3`, which the allowlist accepts. The SHA pin landed in #1335 on Aug 21. ODBC rc2 keeps shipping through exactly this decoupling: its Driver CI run fails the same way, but `build-odbc-packages.yml` is a top-level workflow on `push: release/odbc-**` that never loads `detect-changes`, so the packages still build. JDBC has the same arrangement. `build-python-release.yml` is the Python equivalent and uses no third-party action — it was disabled when Driver CI took over, which is fine on snowflake-eng and leaves the mirror with nothing. So this just turns the push trigger back on: - `push` to `release/python-**`, gated to `snowflakedb`, since a release push on snowflake-eng already gets this matrix from Driver CI at nightly scope - `workflow_dispatch` still works on both repos - `concurrency` per ref, because Copybara lands several commits on a release branch within minutes (rc2 took three pushes in nine minutes today) and each run here is ~76 jobs The wheel targets and the cloud test matrix are untouched. The mirror does have cloud credentials — rc1's [run 32252360737](https://github.com/snowflakedb/drivers/actions/runs/32252360737) was 76 jobs on snowflakedb with aws/gcp/azure legs all green — so the test jobs work there as-is. Unpinning `dorny/paths-filter` in `detect-changes.yml` is worth doing separately, but packaging should not depend on change detection either way: on a `release/python-*` push, `plan` already enables the Python suite from the branch name, so detection contributes nothing. ## Follow-up This does not help `release/python-v5.0.0rc2` until the workflow file exists **on that branch** — GitHub reads workflows from the pushed commit. Once this merges, the release branch needs it cherry-picked (or Copybara needs to carry it) before the next push builds wheels. ## Test plan - Diff is trigger-only: the `on:` block, one `if:` on `build_wheels`, and a `concurrency` block. No job, matrix, or step body changes. - The push gate cannot run on this PR (snowflake-eng, and `pull_request` is not a trigger here), so verification is `workflow_dispatch` on the branch, which the gate's `workflow_dispatch` arm allows on either repo. ## Changelog - No user-visible change; CI packaging only. Co-authored-by: Cursor <cursoragent@cursor.com> GitOrigin-RevId: 697594b8f9c98d1ea078deb8b2ab47b29a50b83a
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds a public
sf_core::bindingsmodule that constructs Snowflake parameter-binding JSON directly from typed Rust values. It introduces aParamValueenum covering every Snowflake logical type the driver binds, and two builders:to_json_singlefor a single parameter set andto_json_arraysfor array (multi-row) binding. The wire-text encoding for each value is byte-for-byte identical to the existing ODBC converters inodbc/src/conversion, so a statement bound through either path yields the same server-side value.Until now
sf_coreaccepted parameter bindings only as pre-serialized JSON bytes:BindingType::Jsonpoints at raw UTF-8 that the core validates and forwards verbatim. Every consumer and test therefore had to hand-assemble the{"1": {"type", "value"}}payload, duplicating the encoding rules and inviting subtle drift from the ODBC behavior. This module gives Rust callers a single typed entry point and one authoritative place for the encoding rules.The design deliberately mirrors the ODBC converters rather than inventing a new format. A few rules that are easy to get wrong are centralized here: every
valueis emitted as a JSON string (never a bare number or boolean),TIMESTAMP_LTZis taggedTEXTand rendered as a bare wall-clock literal,TIMESTAMP_TZencodes epoch nanoseconds plus a+1440-biased offset, and a column that is entirely NULL is taggedANY.INTERVAL_YEAR_MONTHandINTERVAL_DAY_TIMEare accepted as pre-formatted literal strings — matching the ODBCWriteWireimplementations, which are the identity for those types — and the accepted grammar is documented on the enum variants.The primary risk is divergence from the ODBC encoders. That is mitigated by porting each encoding from the corresponding converter and locking the behavior down with tests built from the same expected values the ODBC path produces.
Testing: 43 unit tests cover every logical type, NULL handling, single and array binding, JSON string escaping, and the mixed-type and mismatched-length error paths, asserting on parsed JSON structure rather than key order.
cargo build,cargo test,cargo clippy -D warnings, andcargo fmt --checkare clean for the crate.