-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(snowflake)!: Annotate type for VAR_POP, VAR_SAMP, DuckDB consistency fix for VAR_SAMP #6488
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
Conversation
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.
Thank you for the iterations @fivetran-kwoodbeck, this was tricky once again! One final comment to consider, will go ahead and amend it:
sqlglot/typing/snowflake.py
Outdated
|
|
||
| MAX_PRECISION = 38 | ||
|
|
||
| MIN_SCALE = 12 |
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.
I'd advise against making this a global for now, technically the minimum scale is 0 and we haven't observed anything similar besides VARIANCE iirc.
We can inline this back into the if scale branch and also document that this was found by reverse-engineering Snowflake
| MIN_SCALE = 12 |
Mainly just tests for Snowflake, but in testing I noticed an inconsistency in naming on duckdb. It uses the alias of VAR_SAMP called VARIANCE. I made it consistent so it's VAR_SAMP and VAR_POP, instead of VARIANCE and VAR_POP. sqlglot/dialects/duckdb.py references a VARIANCE_POP which does not seem to exist.