Skip to content

chore(c/validation): add FloatCastTypeName quirk for CAST(... AS FLOAT)#4492

Closed
fornwall wants to merge 1 commit into
apache:mainfrom
fornwall:validation/float-cast-type-quirk
Closed

chore(c/validation): add FloatCastTypeName quirk for CAST(... AS FLOAT)#4492
fornwall wants to merge 1 commit into
apache:mainfrom
fornwall:validation/float-cast-type-quirk

Conversation

@fornwall

@fornwall fornwall commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

TestSqlQueryFloats and TestSqlSchemaFloats hardcoded SELECT CAST(1.5 AS FLOAT), whose FLOAT type name is not valid in every SQL dialect (GoogleSQL in BigQuery supports only FLOAT64 - Spanner also FLOAT32).

Introduce a DriverQuirks::FloatCastTypeName() quirk defaulting to FLOAT and build the cast SQL from it, so existing drivers run byte-identical SQL while other dialects can override the type name. The tests assert only the returned Arrow type and value (1.5), which are unaffected.

@fornwall fornwall requested a review from lidavidm as a code owner July 9, 2026 18:51
@fornwall fornwall force-pushed the validation/float-cast-type-quirk branch from a9f52f3 to ce21e92 Compare July 9, 2026 18:52
@fornwall fornwall changed the title validation: add FloatCastTypeName quirk for CAST(... AS FLOAT) chore(c/validation): add FloatCastTypeName quirk for CAST(... AS FLOAT) Jul 9, 2026
TestSqlQueryFloats and TestSqlSchemaFloats hardcoded SELECT CAST(1.5 AS
FLOAT), whose FLOAT type name is not valid in every SQL dialect (GoogleSQL
in BigQuery supports only FLOAT64 - Spanner also FLOAT32).

Introduce a DriverQuirks::FloatCastTypeName() quirk defaulting to FLOAT and
build the cast SQL from it, so existing drivers run byte-identical SQL
while other dialects can override the type name. The tests assert only the
returned Arrow type and value (1.5), which are unaffected.

Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
@fornwall fornwall force-pushed the validation/float-cast-type-quirk branch from ce21e92 to 010cbf3 Compare July 9, 2026 19:12

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a hook that lets you override entire queries, or just saying that you should override the test, rather than providing a hook just for this one test and driver.

@fornwall

fornwall commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

I'd prefer a hook that lets you override entire queries, or just saying that you should override the test, rather than providing a hook just for this one test and driver.

Thanks - tried a variant in #4496, let me know what you think there.

@fornwall fornwall closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants