Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ parquet = "58.3.0"
serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0", features = ["preserve_order"] }
serde_yaml = "0.9"
# SQL Server TDS client for the sqlserver connector. Crates.io release with
# the spike-proven feature set; its stale rustls 0.21 stack is accepted with
# a weekly cargo-audit sensor carrying the ignore list (ADR-0069).
tiberius = { version = "0.12.3", default-features = false, features = ["tds73", "rustls", "chrono"] }
# SQL Server TDS client for the sqlserver connector. Pinned by full commit
# SHA to the maintainer-controlled fork: the published 0.12.3 sources plus
# one Numeric patch so `decimal(38,38)` loads (ADR-0069 fork rung, #156).
tiberius = { git = "https://github.com/victorchutw/tiberius", rev = "65c1a32fbc8e9c16423a76a76cdb1a800b1c21fa", default-features = false, features = ["tds73", "rustls", "chrono"] }
uuid = { version = "1.10", features = ["v4"] }

[dev-dependencies]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ Every dataset field type maps to the narrowest SQL Server column type that holds

Two mappings are judgment calls rather than forced moves. `utf8` becomes `NVARCHAR(MAX)` because the dataset type is unbounded text and the mapping must stay true under chunked streaming: the writer sees one chunk at a time and can never know a global maximum length, and an append or merge table created from early data must not become invalid when later loads carry longer values. The cost — LOB storage and no plain index on the column — is schema-design territory that belongs to the table's consumers, not the loader. Instant timestamps land as UTC-normalized `DATETIME2(6)` rather than `DATETIMEOFFSET(6)` because the maintainer's production environment stores datetimes exclusively in the `DATETIME` family, so `DATETIME2` is what every downstream consumer already reads; the price, stated plainly, is that wall-clock and instant columns are indistinguishable by destination type — the offset discipline of ADR-0043 lives only in the load definition and the report. Nothing of the instant is lost beyond that marker: ADR-0043 already normalizes instants to UTC at parse without retaining the original offset, so a `DATETIMEOFFSET` column would have carried a constant `+00:00`. Precision is 6, not SQL Server's default 7, because the dataset contract is microseconds (ADR-0043): the column type states the real contract instead of implying 100-nanosecond precision that never arrives.

Because every mapping is exact, a value that satisfies the dataset schema can fail destination typing only in two enumerated slivers: `NVARCHAR` values longer than 32,767 characters — the Tiberius bulk-path client guard measured by the spike (encoded UCS-2 length ≤ 65,535 bytes), a driver-path limit rather than a server one — and timestamps whose value lies outside SQL Server's year range of 0001–9999, reachable because the strict four-digit-year parse menu admits year 0000 and instant UTC normalization can carry a boundary value across the range edge (DuckDB's ±290k-year range never sees either case). These land as write failures, never Rejected Records: a Rejected Record is a record that violates the chosen schema or load rules, and these records violate neither — they exceed one destination path's representable range. Freezing a driver cap into record-level semantics would let the same record load on DuckDB, reject on SQL Server, and load again if the write path ever changes — semantics drifting with implementation. Failure codes, retry classification, and any client-side preflight are write-semantics and error-classification decisions (#109, #112); what an existing table's columns are allowed to look like on append or merge — including production tables whose datetime columns are legacy `DATETIME` — is likewise a named #109 question, not part of this mapping. Non-finite `float64` values are absent from the sliver list because ADR-0063 removes them at parse. Alternatives rejected: `NVARCHAR(4000)` (an arbitrary bound that trades the measured 32,767 cap for a lower, server-side failure surface); length sized from observed data (unknowable under streaming, invalidates evolving append targets); pinning a collation (asserts an opinion the database already answers); `DATETIMEOFFSET(6)` for instants (a self-describing marker, but it buys two bytes per row of type distinction the report already records, against the grain of the environment this tool actually loads into); `DATETIME2(7)` (implied false precision); legacy `DATETIME` as the created type (~3.33 ms rounding against the never-round ethos of ADR-0044, and a 1753 floor below the menu's range); and tightening the parse menu to 0001–9999 (welds one destination's range into the shared contract every destination pays for).
Because every mapping is exact, a value that satisfies the dataset schema can fail destination typing only in two enumerated slivers: `NVARCHAR` values longer than 32,767 characters — the Tiberius bulk-path client guard measured by the spike (encoded UCS-2 length ≤ 65,535 bytes), a driver-path limit rather than a server one — and timestamps whose value lies outside SQL Server's year range of 0001–9999, reachable because the strict four-digit-year parse menu admits year 0000 and instant UTC normalization can carry a boundary value across the range edge (DuckDB's ±290k-year range never sees either case). These land as write failures, never Rejected Records: a Rejected Record is a record that violates the chosen schema or load rules, and these records violate neither — they exceed one destination path's representable range. Freezing a driver cap into record-level semantics would let the same record load on DuckDB, reject on SQL Server, and load again if the write path ever changes — semantics drifting with implementation. Failure codes, retry classification, and any client-side preflight are write-semantics and error-classification decisions (#109, #112); what an existing table's columns are allowed to look like on append or merge — including production tables whose datetime columns are legacy `DATETIME` — is likewise a named #109 question, not part of this mapping. Non-finite `float64` values are absent from the sliver list because ADR-0063 removes them at parse. `decimal(38,38)` is likewise not a third sliver: the released tiberius 0.12.3 panicked on scale 38 and would have described a scale-38 fraction as the invalid `numeric(39,38)`, but that was a dependency defect, not a representability limit — the patched revision ADR-0069 pins (#156) accepts scale 38 and declares such a value as `numeric(38,38)`, so the exact-fit `DECIMAL(p,s)` mapping holds across the complete declaration range. Alternatives rejected: `NVARCHAR(4000)` (an arbitrary bound that trades the measured 32,767 cap for a lower, server-side failure surface); length sized from observed data (unknowable under streaming, invalidates evolving append targets); pinning a collation (asserts an opinion the database already answers); `DATETIMEOFFSET(6)` for instants (a self-describing marker, but it buys two bytes per row of type distinction the report already records, against the grain of the environment this tool actually loads into); `DATETIME2(7)` (implied false precision); legacy `DATETIME` as the created type (~3.33 ms rounding against the never-round ethos of ADR-0044, and a 1753 floor below the menu's range); and tightening the parse menu to 0001–9999 (welds one destination's range into the shared contract every destination pays for).
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ status: accepted

# Take Tiberius from crates.io and Accept Its Stale rustls Stack

The SQL Server destination depends on `tiberius = "0.12.3"` from crates.io, with `default-features = false` and exactly the spike-proven feature set `tds73, rustls, chrono` — #115's type matrix, `DECIMAL` included, round-trips on this set, and `rustls` rather than `native-tls` keeps the single-binary promise, so there is no OpenSSL to vendor. Upstream is alive but has not released since 0.12.3 (2024-07-19; tiberius #321), which made the source a real decision — settled by the spike's build-against-both measurement: git master (`a6b4fcda`, 2026-03-06) is v0.12.3 plus four commits (a vec-reallocation perf tweak, a libgssapi bump behind the unused GSSAPI feature, two chores), byte-near-identical binaries (Δ < 300 B), identical behavior on every probe, and none of the defects this cycle designs around (#322, #410, #425, #329) fixed on either side. The requirement stays caret like every other dependency: the committed `Cargo.lock` plus the `--locked` discipline CI and release already enforce is the actual pin, and any upgrade arrives as a reviewed lockfile diff.
The SQL Server destination originally took `tiberius = "0.12.3"` from crates.io — the source decision this ADR records; the fork activation in the closing paragraph has since replaced the source, and only the source — with `default-features = false` and exactly the spike-proven feature set `tds73, rustls, chrono` — #115's type matrix, `DECIMAL` included, round-trips on this set, and `rustls` rather than `native-tls` keeps the single-binary promise, so there is no OpenSSL to vendor. Upstream is alive but has not released since 0.12.3 (2024-07-19; tiberius #321), which made the source a real decision — settled by the spike's build-against-both measurement: git master (`a6b4fcda`, 2026-03-06) is v0.12.3 plus four commits (a vec-reallocation perf tweak, a libgssapi bump behind the unused GSSAPI feature, two chores), byte-near-identical binaries (Δ < 300 B), identical behavior on every probe, and none of the defects this cycle designs around (#322, #410, #425, #329) fixed on either side. The requirement was caret like every other dependency: the committed `Cargo.lock` plus the `--locked` discipline CI and release already enforce was the actual pin, and any upgrade arrives as a reviewed lockfile diff — a discipline the later git pin keeps, now with the revision also named explicitly in the manifest.

The accepted cost is the stale TLS stack tiberius pins — rustls 0.21.12 + rustls-webpki 0.101.7 + tokio-rustls 0.24.1 (update request tiberius #329 still open) — which `cargo audit` flags with three advisories (tiberius #417). Their reachability in this connector is narrow. RUSTSEC-2026-0104 (reachable panic in CRL parsing) is unreachable: tiberius configures no certificate revocation lists. RUSTSEC-2026-0098 and RUSTSEC-2026-0099 (name-constraint weakening for URI and wildcard names) run only under `encryption: required` with `trust_server_certificate: false` (ADR-0060) — the sole posture that performs webpki path validation — and then only against a CA chain asserting name constraints that a crafted server certificate abuses. The production posture (`encryption: optional`) and the CI posture (`trust_server_certificate: true`) never enter that code. Two scope notes keep the exposure honest: this feature set leaves tiberius's `reqwest`/`oauth2`/`azure_identity` optional tree out entirely, so the flagged crates arrive by exactly one path (`tiberius → tokio-rustls → rustls → rustls-webpki`); and the shipped binary carries exactly one rustls, because the modern rustls 0.23 already in the lockfile belongs to `libduckdb-sys`'s build-dependency graph and never reaches the artifact.

The acceptance stays honest through a sensor and recorded exits. A scheduled weekly, non-blocking `cargo audit` scan of `Cargo.lock` — the three accepted advisories on its ignore list — opens an issue on new findings and never touches the merge gate; it lands through the cycle's slicing ticket (#113) together with the manifest change itself. The source decision reopens on any of: a new advisory reachable in the default TLS path with material severity; an upstream release (taken deliberately, re-running the spike probes); a fix this connector needs merged upstream but unreleased; upstream archival. The response ladder is crates.io first, then a pinned revision on a fork this repository's maintainer controls carrying the minimal patch, then vendoring as the last resort — a bare upstream git pin is never the answer, because it only matters once upstream has merged something, which is exactly when a fork-owned rev is the safer pin.

Alternatives rejected: a pinned git revision today (the spike measured zero benefit, and it would trade away crates.io's checksum and yank protections); an `=0.12.3` requirement (restates what `--locked` already guarantees and breaks the manifest's uniform caret style); forking now to carry the rustls upgrade upstream declines to release (a standing maintenance branch for a risk the deployment postures mostly cannot reach); a blocking advisory gate in CI (an external advisory publication would redden `main` with no code change — the scheduled scan gives the same signal without breaking the always-green gate).

Fork path activated (2026-09-02, #156). The first fix this connector needs arrived before any upstream release: tiberius 0.12.3 asserts `scale < 38` in `Numeric::new_with_scale`, so a `decimal(38,38)` value — valid under ADR-0044's `0 <= s <= p <= 38` and mapped verbatim by ADR-0062 — panics before a bulk row can be emitted, and its `precision()` counts a phantom integral digit for fraction-only values (`1 + scale`), describing a scale-38 fraction as the invalid `numeric(39,38)` in the parameter metadata tiberius emits without destination type context. Upstream `main` (`a6b4fcda`) still carries the assertion; the open hardening PR tiberius-rs/tiberius#436 lifts it but keeps the precision defect, and the optional `rust_decimal` and `bigdecimal` conversions call the same constructor, so no feature choice avoids it. Per the ladder above, the manifest now takes tiberius from the maintainer-controlled fork `victorchutw/tiberius`, pinned by the full commit SHA `65c1a32fbc8e9c16423a76a76cdb1a800b1c21fa` (it sits on branch `data-spark/numeric-scale-38`; the manifest references the revision, never the branch), with `Cargo.lock` recording the same revision; the pin trades crates.io's checksum and yank protection for git's content-addressed revision plus the `--locked` discipline, and CI fetches the public fork anonymously. The base is upstream `c34fab2` ("Bump v0.12.3"): the `v0.12.3` tag plus its own manifest version bump, whose tree is byte-identical to the published 0.12.3 sources (the tag commit itself still declares 0.12.2). The fork carries exactly one patch, confined to `src/tds/numeric.rs` plus test modules (the `column_data.rs` codec tests, `tests/query.rs`, `tests/bulk.rs`): accept `scale <= 38` while still rejecting 39, and compute the precision of a value without integral digits as `max(scale, 1)` — `0.05` is `numeric(2,2)`, zero at scale 0 stays `numeric(1,0)`, a scale-38 fraction is `numeric(38,38)` — while `integral_digits + scale` is preserved when integral digits exist. No dependency, feature, or driver change rides along, so the feature set, the rustls acceptance, and the advisory ignore list above are unchanged. Verification posture: the fork carries its own tests — unit coverage of scale-38 construction, the corrected precision and wire-length buckets, encode/decode round trips, scale-39 rejection, and the `NUMERICNTYPE` metadata bytes, plus live `SELECT @P1` and bulk-insert round trips of `DECIMAL(38,38)` boundary values and fraction-only length-bucket edges against SQL Server 2022 — run locally under the Data Spark feature set before adoption; Data Spark keeps a direct offline smoke test of the pinned revision (`tests/tiberius_dependency.rs`) and operates no separate fork CI, so every future fork revision must pass the same local and live verification before its SHA is adopted, and the fork stays a patch carrier rather than tracking upstream `main`. Upstream trace: the reproduction and the complete invariant were posted on tiberius-rs/tiberius#436 (https://github.com/tiberius-rs/tiberius/pull/436#issuecomment-5505639544); a merge or release upstream is not a condition of this activation. Exit condition: return to crates.io — the first rung — as soon as an upstream release contains both halves of the fix and passes the same Data Spark probes, retiring the fork branch.
55 changes: 55 additions & 0 deletions tests/tiberius_dependency.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//! Direct smoke test of the pinned Tiberius dependency (#156, ADR-0069).
//!
//! Data Spark's declared Decimal contract admits `decimal(38,38)`
//! (ADR-0044), and the SQL Server mapping carries that declaration
//! verbatim (ADR-0062). Tiberius 0.12.3 from crates.io panics on scale 38
//! and describes a fraction-only scale-38 value as `numeric(39,38)`, so
//! the manifest pins a maintainer-controlled fork carrying the fix. This
//! test proves the pinned revision honors the complete scale-38 invariant
//! without opening a connection; the live round trip lives in the fork's
//! own test suite.

use tiberius::numeric::Numeric;
use tiberius::{ColumnData, ToSql};

/// The #134 regression seed: Arrow `Decimal128(38,38)` scaled value `-1`.
const SCALED_MINUS_ONE: i128 = -1;

#[test]
fn pinned_tiberius_builds_the_scale_38_value_deterministically_with_precision_38() {
let first = Numeric::new_with_scale(SCALED_MINUS_ONE, 38);
let second = Numeric::new_with_scale(SCALED_MINUS_ONE, 38);

for value in [first, second] {
assert_eq!(value.value(), SCALED_MINUS_ONE);
assert_eq!(value.scale(), 38);
assert_eq!(value.int_part(), 0);
assert_eq!(value.dec_part(), SCALED_MINUS_ONE);
assert_eq!(
value.precision(),
38,
"a fraction-only scale-38 value is numeric(38,38), never numeric(39,38)"
);
}

assert_eq!(first.to_sql(), ColumnData::Numeric(Some(second)));
}

#[test]
fn pinned_tiberius_reports_precision_38_across_the_scale_38_range() {
let max_magnitude = 10i128.pow(38) - 1;

for value in [0, 1, max_magnitude, -max_magnitude] {
let numeric = Numeric::new_with_scale(value, 38);
assert_eq!(numeric.value(), value);
assert_eq!(numeric.scale(), 38);
assert_eq!(numeric.precision(), 38, "precision of {value} at scale 38");
}
}

#[test]
#[should_panic(expected = "scale <= 38")]
fn pinned_tiberius_still_rejects_scale_39() {
// Scale 39 exceeds SQL Server's cap and must keep panicking.
Numeric::new_with_scale(1, 39);
}