Skip to content
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

fix(backends): ensure that analytic functions do not receive a window frame #10739

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Jan 28, 2025

Fixes #10699.

@cpcloud cpcloud added bug Incorrect behavior inside of ibis bigquery The BigQuery backend ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests labels Jan 28, 2025
@github-actions github-actions bot added tests Issues or PRs related to tests sql Backends that generate SQL labels Jan 28, 2025
@ibis-docs-bot ibis-docs-bot bot removed the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Jan 28, 2025
@cpcloud cpcloud force-pushed the fix-bigquery-windows branch from dd93db8 to 7a7d883 Compare January 28, 2025 15:07
@cpcloud cpcloud added this to the 10.0 milestone Jan 28, 2025
@cpcloud
Copy link
Member Author

cpcloud commented Jan 28, 2025

This turns out to be an issue on a number of existing backends, and I fixed as many as I could fix easily.

@cpcloud
Copy link
Member Author

cpcloud commented Jan 28, 2025

So far, ClickHouse and MySQL are the outliers: their analytic function behavior seems to differ when given a window frame clause.

@cpcloud
Copy link
Member Author

cpcloud commented Jan 28, 2025

Ok, I think I managed to address clickhouse and mysql as well. Working on the impala snapshots now.

@github-actions github-actions bot added the impala The Apache Impala backend label Jan 28, 2025
@cpcloud cpcloud force-pushed the fix-bigquery-windows branch from 35c8f79 to c8471d3 Compare January 28, 2025 16:07
@cpcloud cpcloud added clickhouse The ClickHouse backend snowflake The Snowflake backend mysql The MySQL backend mssql The Microsoft SQL Server backend labels Jan 28, 2025
@cpcloud cpcloud force-pushed the fix-bigquery-windows branch from c8471d3 to fe0cd56 Compare January 28, 2025 16:39
@cpcloud cpcloud added the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Jan 28, 2025
@ibis-docs-bot ibis-docs-bot bot removed ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests labels Jan 28, 2025
@cpcloud cpcloud force-pushed the fix-bigquery-windows branch 2 times, most recently from a19c3b5 to 876c2f4 Compare January 28, 2025 17:34
@cpcloud cpcloud force-pushed the fix-bigquery-windows branch from 876c2f4 to 96214df Compare January 28, 2025 17:37
@cpcloud
Copy link
Member Author

cpcloud commented Jan 28, 2025

Ok, this turned out to address a good chunk of the inconsistency with window function behavior across backends. We're not at 100% consistency, but this is a decent improvement.

Copy link
Member

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

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

Nice! I'm going to edit the PR title so we get the broader scope captured in the changelog. I'll leave this for you to merge in case you have any extra tinkering you'd like to do.

@gforsyth gforsyth changed the title fix(bigquery): ensure that analytic functions do not receive a window frame fix(backends): ensure that analytic functions do not receive a window frame Jan 28, 2025
@cpcloud cpcloud merged commit 85d5d68 into ibis-project:main Jan 29, 2025
87 checks passed
@cpcloud cpcloud deleted the fix-bigquery-windows branch January 29, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bigquery The BigQuery backend bug Incorrect behavior inside of ibis clickhouse The ClickHouse backend impala The Apache Impala backend mssql The Microsoft SQL Server backend mysql The MySQL backend snowflake The Snowflake backend sql Backends that generate SQL tests Issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: cumsum function does not work as expected in Bigquery backend due to ibis default window frame logic
2 participants