chore(c/validation): add RewriteSql quirk to override test queries#4496
Open
fornwall wants to merge 2 commits into
Open
chore(c/validation): add RewriteSql quirk to override test queries#4496fornwall wants to merge 2 commits into
fornwall wants to merge 2 commits into
Conversation
TestSqlQueryFloats and TestSqlSchemaFloats hardcoded SELECT CAST(1.5 AS FLOAT), whose FLOAT type name is not valid in every SQL dialect (GoogleSQL in BigQuery accepts only FLOAT64 - Spanner also FLOAT32). Add a generic DriverQuirks::RewriteSql(query_id, default_sql) hook. It is keyed by a stable query id of the form <test>::<description> (e.g. StatementTest::TestSqlQueryFloats::cast-1.5-as-float) so driver overrides switch on the id rather than matching literal query text, and takes the default query as an argument so the base method just returns it - existing drivers run byte-identical SQL. Drivers whose dialect differs can override the whole query. Wire it into the two float-cast select tests to start with; the assertions only check that the result is an Arrow float or double equal to 1.5, which is unaffected. Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
Contributor
Author
|
This is another approach compared to #4492 for the C++ validation suite requring
@lidavidm What do you think of this approach? |
lidavidm
reviewed
Jul 9, 2026
Comment on lines
+165
to
+168
| virtual std::string RewriteSql(std::string_view query_id, | ||
| std::string_view default_sql) const { | ||
| return std::string(default_sql); | ||
| } |
Member
There was a problem hiding this comment.
nit: should probably just take default_sql as std::string
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.
TestSqlQueryFloatsandTestSqlSchemaFloatshardcodedSELECT CAST(1.5 AS FLOAT), whoseFLOATtype name is not valid in every SQL dialect (GoogleSQL in BigQuery accepts onlyFLOAT64- Spanner alsoFLOAT32).Add a generic
DriverQuirks::RewriteSql(query_id, default_sql)hook. It is keyed by a stable query id of the form<test>::<description>(e.g.StatementTest::TestSqlQueryFloats::cast-1.5-as-float) so driver overrides switch on the id, and takes the default query as an argument so the base method just returns it.Drivers whose dialect differs can override the whole query. Wire it into the two float-cast select tests to start with to fix the
FLOATissue - it can be inserted into other tests on demand.