Add adapter conformance test suite#2664
Open
paulteehan wants to merge 4 commits into
Open
Conversation
…lect) 67 integration tests that validate every adapter handles identifier quoting, metadata discovery, type mapping, sampling, and regex correctly. Run against all 10 adapters: 620 passed, 46 skipped, 1 failure (Redshift float synonym). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
for more information, see https://pre-commit.ci
paulteehan
force-pushed
the
feat/adapter-conformance-tests
branch
from
8c576f3 to
0de0c27
Compare
AI-written. Adapter source fixes (real bugs surfaced by the synonym bidirectionality test): - soda-redshift: move "float" out of the FLOAT synonym group and into the DOUBLE group. In Redshift, FLOAT is an alias for FLOAT8 / DOUBLE PRECISION, not for REAL / FLOAT4. The reverse mapping already had this right (float -> DOUBLE); the synonym group was inconsistent. - soda-duckdb: split the catch-all integer/decimal group and the catch-all single+double precision float group into proper per-type synonym groups. Drop type names that don't exist in DuckDB (number, byteint, timestamp_ntz, timestamp_ltz, timestamp_tz). Test fixes: - test_conformance_discovery: relax datetime_precision check from == N to >= N. Trino connectors (e.g. iceberg) normalize datetime precision to a connector-specific default (often 6) regardless of DDL, which still satisfies the contract. Drop unused test_table local in the internal-table filter test. Remove unused f-string prefixes and add comments where dialect-internal accessors are needed. - test_conformance_identifiers: case-insensitive identifier preservation check; some dialects fold case during quoting. Drop gratuitous "is not None" assertions on values typed as str. - test_conformance_types_dialect: drop two redundant private-API tests (_build_sample_sql, _build_regex_like_sql) — coverage is preserved by the existing end-to-end tests in the same file. Remove gratuitous always-true assertions (assert result is not None on a typed return, assert len(rows) >= 0). Clean up unused imports. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI black hook collapsed two adjacent f-string literals onto one line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
m1n0
approved these changes
Apr 30, 2026
Contributor
m1n0
left a comment
m1n0
left a comment
There was a problem hiding this comment.
NIT: conformance without context isn't clear, I had to skim through tests to understand the motivation of this PR. Would it make sense to a) rename or b) put these tests in a descriptive folder?
m1n0
reviewed
Apr 30, 2026
Contributor
m1n0
left a comment
m1n0
left a comment
There was a problem hiding this comment.
One more thought @paulteehan - we are adding a whole bunch if e2e tests here, I am a bit worried about bloating the test suite. A lot of these e2e tests are very similar to check type tests (e.g. missing etc) just with the variation in table/column name using a special character or something like that. Wouldn't it make sense to either a) consolidate these into less tests overall or b) unit test generated queries to make sure identifiers are handled correctly?
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
2 participants
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.
Summary
AI-written.
Adapter conformance test suite — 65 integration tests across 3 new files that catch field bugs at the adapter contract layer. Plus two adapter source fixes for real bugs the suite found.
test_conformance_identifiers.py(24 tests) — reserved SQL words, hyphens, mixed-case as column names; end-to-end DDL+DML+contract checks; parametrized quoting consistency.test_conformance_discovery.py(9 tests) — internal object filtering, view/MV contract checks, type round-trip for all SodaDataTypeNames, synonym bidirectionality, column type parameters (precision/scale/length).test_conformance_types_dialect.py(32 tests) — missing check per data type, aggregate per numeric type, schema check, sampling SQL execution per sampler, regex via invalid check, RANDOM() value range, forward/reverse type mapping, synonym consistency.Designed to catch the top 3 categories of field bugs: identifier quoting (~30%), metadata discovery (~20%), and type mapping/dialect (~18%).
Bugs found and fixed
While running this suite across all adapters, the synonym bidirectionality test found two real bugs in the synonym definitions. Both are fixed in this PR:
soda-redshift/.../redshift_data_source.py):floatwas declared a synonym ofreal/float4, but its reverse mapping correctly resolves it toDOUBLE(it is an alias forfloat8/double precision). Fix: movefloatout of["real", "float4"]and into[REDSHIFT_DOUBLE_PRECISION, "float8", "float"].soda-duckdb/.../duckdb_data_source.py): two synonym groups lumped together types that map to different SodaDataTypes — all integer types were grouped withdecimal/numeric, and single-precision floats were grouped with double-precision floats. Fix: split into nine correct synonym groups; drop names that aren't DuckDB types (number,byteint,timestamp_ntz,timestamp_ltz).Other changes from review
datetime_precision == Nto>= Nintest_column_type_parameters_preserved. Trino connectors (e.g. iceberg) normalize datetime precision to a connector-specific default (often 6) regardless of DDL — the relaxed check still catches "no precision returned at all" while accepting normalization.test_ddl_and_dml_quoting_both_preserve_identifier— some dialects fold case during quoting.test_sampling_sql_generation,test_regex_sql_generation) — coverage is preserved by the end-to-end tests in the same file. Remaining private-API uses (_get_data_type_name_synonyms,_data_type_name_synonym_mappings) are kept with explanatory comments because no public accessor exists for the synonym table itself.assert result is not Noneon a typed return,assert len(rows) >= 0).Results per adapter (latest CI on this branch)
To be updated once CI completes. Prior run was 620 passed / 46 skipped / 1 failed (Redshift float synonym, now fixed). DuckDB and Trino-S3 also failed and are now fixed.
Test plan
🤖 Generated with Claude Code