Skip to content

fix(sql): replace QUALIFY with ANSI-standard subquery wrapper - #17

Merged
aleattene merged 4 commits into
mainfrom
chore/verify-ansi-sql
Apr 27, 2026
Merged

aleattene merged 4 commits into
mainfrom
chore/verify-ansi-sql

Conversation

@aleattene

Copy link
Copy Markdown
Owner
  • QUALIFY is supported by DuckDB/BigQuery/Snowflake but is not part of the ANSI SQL standard and is rejected by PostgreSQL, MySQL, SQL Server.
  • Replace QUALIFY ROW_NUMBER() = 1 with a subquery and WHERE rn = 1 pattern in q_bq2 and q_bq3 first-assessment subqueries.
  • Functionally identical but fully portable to any ANSI-compliant engine.

- QUALIFY is supported by DuckDB/BigQuery/Snowflake but is not part of the ANSI SQL standard and is rejected by PostgreSQL, MySQL, SQL Server.
- Replace QUALIFY ROW_NUMBER() = 1 with a subquery and WHERE rn = 1 pattern
  in q_bq2 and q_bq3 first-assessment subqueries.
- Functionally identical but fully portable to any ANSI-compliant engine.
@codecov-commenter

codecov-commenter commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.53%. Comparing base (8c032f5) to head (2b6b295).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #17   +/-   ##
=======================================
  Coverage   93.53%   93.53%           
=======================================
  Files           9        9           
  Lines         325      325           
=======================================
  Hits          304      304           
  Misses         21       21           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR removes use of the non-ANSI QUALIFY clause in BQ2/BQ3 queries by rewriting “first assessment” selection logic to an ANSI-style derived table filtered with WHERE rn = 1, improving portability across SQL engines.

Changes:

  • Replaced QUALIFY ROW_NUMBER() = 1 with a subquery wrapper and WHERE rn = 1 in BQ2 first-assessment subquery.
  • Replaced QUALIFY ROW_NUMBER() = 1 with a subquery wrapper and WHERE rn = 1 in BQ3 first-assessment subquery.
  • Added explanatory comments about portability and rationale.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
sql/queries/q_bq2_early_signals.sql Rewrites first-assessment selection to avoid QUALIFY by using a ranked subquery and filtering rn = 1.
sql/queries/q_bq3_demographics_vs_behavior.sql Applies the same QUALIFY-removal pattern for first-assessment selection and updates inline documentation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sql/queries/q_bq3_demographics_vs_behavior.sql Outdated
Comment thread sql/queries/q_bq3_demographics_vs_behavior.sql Outdated
Comment thread sql/queries/q_bq2_early_signals.sql Outdated
- Remove redundant FIRST_VALUE windows: on the rn = 1 row the direct column values are already the first-submitted values
- Fix misleading "Same subquery as BQ2" comment in q_bq3 (BQ3 only selects first_score, not first_submit_day)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sql/queries/q_bq2_early_signals.sql Outdated
Comment thread sql/queries/q_bq2_early_signals.sql
Comment thread sql/queries/q_bq3_demographics_vs_behavior.sql Outdated
Comment thread sql/queries/q_bq3_demographics_vs_behavior.sql
First CMA assessments had date=30 but BQ2/BQ3 filter date <= 28, so the rn=1 selection was never tested.
Change date to 14 for all 6 course-presentations and add coverage tests.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@aleattene
aleattene merged commit 1859808 into main Apr 27, 2026
6 checks passed
@aleattene
aleattene deleted the chore/verify-ansi-sql branch April 27, 2026 08:15
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.

3 participants