Repository navigation
Conversation
Backtick quote_ident (5 dialects), the 1/0 boolean literal (3), and the two identical sql_temporal_as_number pairs (Postgres/Redshift, DataFusion/MonetDB) now live as shared helpers in dialects/mod.rs.
Dialects are stateless unit structs, so boxing a fresh trait object per lookup was pointless churn. The registry now stores &'static dialect references (DialectRef) and resolve_dialect/detect_or_err/ dialect_override return them; reader structs and test stubs hold the reference directly. The dialect conformance sweep iterates REGISTRY instead of a hand-copied ALL_SCHEMES list, so new backends are covered automatically.
Two behaviors previously changed globally to accommodate Oracle are now capability-driven: - NUMERIC/DECIMAL with scale 0 and <= 15 digits binds as Int64 again (preserving integer type and precision past 2^53) except where the registry declares odbc_numeric_as_double (Oracle, HY090). - The per-cell text buffer is sized from the driver-reported column size (capped at 1 MiB, x4 for UTF-8) instead of a flat 16 KiB, so longer values in sized columns fit; unsized columns keep the default and overlong values remain an explicit error, never truncated.
- battery::fixture_df is now the single Arrow fixture builder (golden tier and the DataFusion live leg); BigQuery's hand-built batch stays, documented as needing Int64 ids. - Unit tests for StubReader's SQL-scanning helpers (select_list_span, split_top_level_commas, output_name), previously only exercised indirectly through goldens. - registry.rs gains an 'Adding a dialect' checklist covering all ~7 touch points; dialect_live.rs documents that Drill is golden-only.
dialect-live.yml and dialect-live-cloud.yml carried near-verbatim copies of the compile job (already drifting: save-if present in one, missing in the other, so the nightly could fight over the shared cache key). The job now lives in dialect-live-compile.yml via workflow_call, with the save decision as an explicit input.
auto_cache_if_needed reuses cache_uri() instead of a second spelling of the in-memory URIs (which had drifted: 'sqlite://:memory:' vs 'sqlite://memory'). The two cache-disable knobs (cache=off vs cache_disabled) and the reader= param's accepted values are now documented where they bite, as is the deliberate probe/build double dlopen of ADBC drivers.
thomasp85
added this pull request to stack #579
October 6, 2026 17:51
- Remove redundant &* reborrows now that readers hold DialectRef - Drop unnecessary usize cast in ODBC text-buffer sizing - Update ggsql-jupyter data explorer to pass FromItem::Query - Remove unused Arc import; move assert_dataframes_equal above the test module (items_after_test_module)
This branch has not been deployed
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.
Stacked on #577 (itself stacked on #569). Second wave of review follow-ups — mostly mechanical deduplication, all tests green (2095 lib tests, goldens unchanged).
quote_ident(5 dialects), 1/0 boolean literal (3), and the two identicalsql_temporal_as_numberpairs are now shared helpers indialects/mod.rs.&'staticdialect instances (DialectRef) instead of boxing per lookup; the conformance sweep iteratesREGISTRYinstead of a hand-copied scheme list, so new backends are covered automatically.odbc_numeric_as_double(Oracle); text buffers size from the driver-reported column size (1 MiB cap) instead of a flat 16 KiB, with overlong values still an explicit error.battery::fixture_dfis the single fixture builder; StubReader's SQL-scanning helpers have direct unit tests;registry.rsdocuments the full 'adding a dialect' checklist; Drill's golden-only status is documented.workflow_callworkflow, fixing thesave-ifcache drift between the PR and nightly workflows.Deliberately skipped (from the plan): the geom
__ggsql_*__alias-boilerplate helper and the five 'numbered rows × cross join' fragment dedups — cosmetic, consistent as-is, and not worth another golden churn cycle; happy to do it as a separate small PR if wanted. Thee2e.rssubmodule reorg is inherited structure, also deferred.