Generate SQL Server DDL and bulk rows from the dataset schema - #158
Merged
Conversation
Add the offline SQL Server type-mapping module of ADR-0062: the created shape as CREATE TABLE DDL with exact-fit column types, mirrored nullability, bracket-quoted identifiers, and no COLLATE clause; and a BulkRowPlan that encodes each chunk's records into tiberius TokenRows in destination table column order, preserving NULLs, keeping empty strings present, carrying microsecond DATETIME2(6) values built directly at scale 6, and emitting decimals as Numeric at the declared scale (the decimal(38,38) scaled -1 regression from #156 is pinned). The encoder refuses a timestamp outside DATETIME2's 0001-9999 range as a write failure rather than reaching tiberius, whose Date::new asserts on a pre-0001 day count instead of returning an error; a panic is not the write failure ADR-0062 assigns that sliver. Over-long NVARCHAR values are not preflighted: tiberius reports them at send(). Define Created Shape in the glossary, a term ADR-0062/0064/0065 already use. No CHANGELOG entry: nothing is user-observable until a mode goes live (#136). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Promote the write-failure constructor to one module-level helper, spell out-of-range timestamps in the dataset's own notation instead of chrono's Debug output, state that the DATETIME2 upper bound is the lower bound's rule applied symmetrically (only the pre-0001 side is forced by tiberius's assert), note that plan mismatches are invariant breaches behind the Accept Family rather than table validation, and drop "Generated DDL" from the Created Shape avoid list because ADR-0062 uses that phrase itself. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
變更為離線純模組且以單元測試完整覆蓋型別對映、識別子 escaping、欄位順序與邊界值,未發現需阻擋合併的問題。
Pull request overview
這個 PR 在 CLI/core 增加一個純離線(不開連線)的 SQL Server 映射模組,將 dataset schema 轉為「Created Shape」的 CREATE TABLE DDL,並把 RecordBatch 編碼成 tiberius bulk insert 所需的 TokenRow(依目的地 table 欄位順序),以供後續 #136 的 live connector 消費。
Changes:
- 新增
src/sqlserver.rs:實作create_table_ddl(精確型別對映與識別子 bracket quoting)與BulkRowPlan/rows()(table 欄位順序的 bulk rows 編碼),並以單元測試覆蓋 DDL 與 row 編碼矩陣與邊界案例。 src/lib.rs註冊sqlserver模組,並以#[cfg_attr(not(test), allow(dead_code))]避免在尚無 live consumer 前觸發 dead_code。CONTEXT.mdGlossary 新增「Created Shape」定義,與 ADR/後續 slice 的術語一致。
File summaries
| File | Description |
|---|---|
| src/sqlserver.rs | 新增 SQL Server DDL 生成與 bulk row 編碼(含完整單元測試覆蓋)。 |
| src/lib.rs | 註冊 sqlserver 模組並暫時抑制非測試建置的 dead_code。 |
| CONTEXT.md | Glossary 補上「Created Shape」術語定義以對齊 ADR 用語。 |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The maintainer ruled on PR #158: keep the encoder's symmetric 0001-9999 range check, and record it on the ADR that owns the sliver rather than leaving the decision in code alone. The note states what forced the lower bound (tiberius asserts on a pre-0001 day count), why the upper bound follows the same rule, and why this is a write failure rather than a preflight or a Rejected Record. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner
Author
|
Merge authorization: the maintainer authorized the agent to merge this PR in the active agent session on 2026-09-03 (instruction: "merge"). Required checks |
This was referenced Sep 3, 2026
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
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.
Closes #134
Implements the mechanics of ADR-0062 as a pure, offline module (
src/sqlserver.rs): the created shape a dataset schema takes asCREATE TABLEDDL, and the encoding of one chunk's records into tiberius bulk rows in destination table column order. Nothing opens a connection; every test is an ungated unit test (ADR-0066).What lands
create_table_ddl(dataset, schema, table)—CREATE TABLE [schema].[table] (...)with the exact-fit mapping:int64→BIGINT,float64→FLOAT(53),boolean→BIT,utf8→NVARCHAR(MAX),timestamp/timestamptz→DATETIME2(6),decimal(p,s)→DECIMAL(p,s)verbatim. Nullability mirrors the field (NULL/NOT NULL), identifiers are bracket-quoted with]]escaping, and noCOLLATEclause is emitted. A schema with no fields, or an Arrow type outside the dataset vocabulary, is adestination_write_failednaming the field — never an approximate mapping.BulkRowPlan::new(dataset, table_columns)/rows(&RecordBatch)— oneTokenRowper record, columns in the table order the caller states (one-to-one with the dataset fields by exact name; unknown, duplicate, or missing columns fail the plan).NULLs areNoneinside the typed variant, an empty string is a present value, instants ride as the UTC value ADR-0043 already stored,DATETIME2values are built directly at scale 6 (day count from 0001-01-01 + microseconds into the day, so tiberius writes them as-is with no rescale), and decimals becomeNumeric::new_with_scale(value, declared_scale)— the scale the created shape carries and the Accept Family (ADR-0065) requires of an existing column, since tiberius encodes a value only at the column's own scale.CONTEXT.mdgains Created Shape, a term ADR-0062/0064/0065 and Validate existing SQL Server tables against the Accept Family #135 already use without a definition.lib.rsregisters the module under#[cfg_attr(not(test), allow(dead_code))]until Land the sqlserver connector: session and full-refresh loads #136 consumes it, mirroring the precedent onSqlServerConfig.One judgement call to review: the
DATETIME2rangeThe issue says the two representability slivers (over-long
NVARCHAR, timestamps outside 0001–9999) get no client-side preflight in this module. That holds forNVARCHAR: values reach the wire and tiberius returnsErr(BulkInput(...))atsend(), a clean chunk write failure.It cannot hold for timestamps. Tiberius's
Date::newasserts (days >> 24 == 0, forksrc/tds/time.rs:168) instead of returning an error, so a pre-0001 value — reachable through the parse menu (0000-…wall-clock) or instant normalization (0001-01-01T00:00:00+01:00→0000-12-31T23:00:00Z) — would panic inside row encoding, and a panic is not the write failure ADR-0062 assigns that sliver. The encoder therefore refuses a timestamp outside 0001-01-01..=9999-12-31 with adestination_write_failednaming the record, field, value, and range (rows_fail_instead_of_panicking_for_timestamps_outside_the_datetime2_range).Scope, stated honestly: only the lower bound is forced by the driver. Years 10000–45941 would pass tiberius's asserts and reach the server as a genuine write failure; the upper bound is the same rule applied symmetrically, so the refused range is exactly ADR-0062's enumerated sliver with one message, rather than a driver-shaped subset. It is the conversion reporting what it cannot produce, not a separate validation pass, and the live slice maps it to a chunk write failure like any other.
Ruling (maintainer, 2026-09-03): keep the symmetric range check as implemented, and record it on ADR-0062 as a dated "Range check activated" note in the ADR-0069 pattern, in this PR. The note is the third commit; the code is unchanged.
Acceptance criteria
ddl_renders_every_type_and_nullability_combination_exactly(9 type cases × 2 nullabilities),ddl_doubles_closing_brackets_inside_every_identifier(]in column, schema, and table names),ddl_never_carries_a_collate_clause, field order kept, empty schema and unmapped types fail.rows_emit_every_type_in_table_column_order_not_dataset_order(seven columns, table order ≠ field order, with hand-derived day counts),rows_keep_an_empty_string_distinct_from_null,rows_carry_the_numeric_boundaries_exactly(i64::MAX/i64::MIN,f64::MAX/f64::MIN/f64::MIN_POSITIVE, ±(10³⁸−1)),rows_carry_negative_decimals_with_the_declared_scale,rows_carry_the_datetime2_range_edges_and_microseconds_exactly(0001-01-01T00:00:00 and 9999-12-31T23:59:59.999999,.000001and.999999), plusrows_agree_with_the_tiberius_chrono_conversion— the direct construction cross-checked against tiberius's own chrono path (the one the Spike: validate Tiberius against a containerized SQL Server #115 spike round-tripped through a real server) at ten calendar edges, and the day constants checked against chrono's proleptic Gregorian calendar.rows_convert_decimal_38_38_scaled_minus_one_to_a_scale_38_numeric: ArrowDecimal128(38,38)scaled-1→Numericvalue-1, scale 38,precision() == 38, encoded twice with equal results.#[ignore].CHANGELOG.mdentry: nothing is user-observable until Land the sqlserver connector: session and full-refresh loads #136 goes live.Review
Two-axis
/code-review(Standards × Spec) overgit diff main...HEAD, both reports valid. Standards: no documented-standard violation; the judgement calls acted on are a module-levelwrite_failurehelper (six literal constructions collapsed), out-of-range timestamps rendered in the dataset's own notation instead of chrono'sDebug, a doc note that plan mismatches are invariant breaches behind the Accept Family rather than table validation, and the glossary avoid list no longer names "Generated DDL" because ADR-0062 uses that phrase. Left as-is: the fourmatch ColumnTypecascades (idiomatic),schema_name/table_nameas two&str(aQualifiedTabletype can be born when #135/#136 need it), andtable_columns: &[String](the seam #135 will type). Spec: AC1–AC5 each traced to a test; day constants, scale handling, null handling, and table order independently re-derived; the only deviation is the timestamp range above, judged justified, with the scope note now in the module docs. Hand-off for #136: the fork's bulkNumericarmtodo!()-panics on a row/column scale mismatch, so ADR-0065 validation must run before anybulk_insert.Local checks
cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings,cargo test --locked, andcargo build --release --locked --bin data-sparkall pass.Seams left for the next slices
IDENTITY/ defaulted, ADR-0065) need the introspected column type to choose their placeholder, so they join the plan with Validate existing SQL Server tables against the Accept Family #135/Land the sqlserver connector: session and full-refresh loads #136; this slice plans full-schema rows only.🤖 Generated with Claude Code