-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(snowflake)!: Added UNIFORM transpilation for Snowflake to DuckDB #6640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
SQLGlot Integration Test ResultsComparing:
By Dialect
Overallmain: 6173 total, 5610 passed (pass rate: 90.9%), sqlglot version: sqlglot:feature/transpile-uniform: 6173 total, 5610 passed (pass rate: 90.9%), sqlglot version: Difference: No change |
VaggelisD
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work once again @fivetran-kwoodbeck! Though extreme, I was wondering whether DuckDB's RANDOM() is enough to simulate uniform distributions, but it's probably fine.
Leaving a few comments to consider:
sqlglot/dialects/duckdb.py
Outdated
| UNIFORM_INT_TEMPLATE: exp.Expression = exp.maybe_parse( | ||
| "CAST(FLOOR(:min + :random * (:max - :min + 1)) AS BIGINT)" | ||
| ) | ||
|
|
||
| UNIFORM_FLOAT_TEMPLATE: exp.Expression = exp.maybe_parse(":min + :random * (:max - :min)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't really have to parse these as templates, it should be doable to generate these expressions directly.
Do note that SQLGlot expressions have overloaded magic methods, meaning that math ops between expressions are automatically parsed as subtrees:
>>> foo
Var(this=foo)
>>> math_expr = foo + foo * (foo - 1)
>>> math_expr
Add(
this=Var(this=foo),
expression=Paren(
this=Mul(
this=Var(this=foo),
expression=Paren(
this=Sub(
this=Var(this=foo),
expression=Literal(this=1, is_string=False))))))
>>> math_expr.sql()
'foo + (foo * (foo - 1))'There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I originally did it as expressions but thought it might have been too complex. I reverted, let me know if that works.
|
I did some testing across 100 samples for a variety of parameters, it's in the Jira. RANDOM seems to work OK.
|
Added transpilation of Snowflake UNIFORM.
Changes
sqlglot/dialects/duckdb.py
tests/dialects/test_snowflake.py
Transpilation Logic
Snowflake's UNIFORM(min, max, gen) generates random values in the range [min, max]:
Implementation Details