Repository navigation
Conversation
…, sqlite, redshift
Collaborator
Author
|
I've been hard at work setting up testing infrastructure for this so we can be somewhat sure that ggsql actually works with the stated backends. For non-cloud backends we have a live CI that runs a battery of queries against a db. For cloud based we have a nightly run backed by credentials in the repo secrets The tests currently miss:
All of this testing has payed off. So many portability issues have been uncovered and fixed at the detriment of the complexity of this PR. Some stat queries have been rewritten, the dialect system expanded, and quoting has become a part of the dialect |
- Error instead of panicking when a scheme without registry ADBC info (monetdb, drill) reaches AdbcReader::from_connection_string, e.g. via the GGSQL_<SCHEME>_ADBC_DRIVER env override. - reader=native no longer falls through to ODBC when no native reader exists or is compiled in; it errors explicitly like reader=adbc. - odbc:// URIs are handed to the driver with ggsql-owned params stripped, so ?cache=off and friends no longer leak into the connection string.
CachingReader::materialize_table executes the body against the primary but quoted aliases with the cache dialect, sending double-quoted identifiers to backtick dialects like MySQL. Regression test included.
- case_greatest/case_least no longer index-panic on empty input. - Date/datetime/time literal defaults clamp out-of-range input instead of panicking (and no longer wrap negative time-of-day via as u32). - drop_stmt_handle skips its debug_assert while unwinding, avoiding an abort on a failed free during panic. - ODBC diagnostic buffer growth is clamped to SqlSmallInt::MAX so the length argument cannot wrap negative.
- Dialect module docs for redshift, exasol, monetdb and bigquery claimed spatial support that supports_spatial() (false by default) blocks; the docs now match the code. - Battery live_skip for the boxplot global-source case extended to mariadb, which shares MySQL's temp-table reopen limitation. - Rename leftovers (sql_percentile -> sql_quantile) and stale doc references cleaned up. - CHANGELOG entries for the API breaks and behavior changes riding along in this PR.
This was referenced Oct 6, 2026
thomasp85
added this pull request to stack #579
October 6, 2026 17:51
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.
This PR provides a large swath of readers build on top of the existing ADBC/ODBC support we have. It refactors the dialects into a specific module with dialect detection, expands the dialect to accomodate new wrinkles discovered by the new readers, provides a multitiered selectioon mechanism where ADBC is preferred, but ODBC is tried if no suitable ADBC driver exist, and lastly adds an autowrapping of a cache reader when needed. The latter is supported through the dialect system as some dbs forcefully requires a cache to work. For the others we try a temp table creation and if it fails we use a cache.
The dialects added here are
Fix #341, fix #509, closes #386, closes #458
Changes with runtime-semantics impact riding along in this PR
These commits change behavior rather than just how SQL text is built. They are
called out here so they can be reviewed individually; see also the CHANGELOG
entries.
a687c9c4— Vega-Lite timestamp double-rescaling fix(
src/writer/vegalite/data.rs): unrelated correctness fix for microsecondtimestamps being rescaled twice.
resolution (
src/plot/scale/scale_type/mod.rs,temporal_unit_micros,convert_range_to_transform_unit,check_temporal_domain): new error path,surfaced by the broader multi-backend test matrix.
src/execute/schema.rs,merge_extent_rows):portability fix for MySQL "can't reopen table" / ClickHouse alias reuse;
changes how min/max extents are derived.
CacheBackendmoved to test-support,CachingReader::newis now
#[cfg(test)],Spec::layer_sql/Spec::stat_sqlremoved. Recorded inCHANGELOG under [Unreleased].