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

Update SqlWindowFunction enum #1266

Merged
merged 2 commits into from
Jun 12, 2024
Merged

Update SqlWindowFunction enum #1266

merged 2 commits into from
Jun 12, 2024

Conversation

courtneyholcomb
Copy link
Contributor

@courtneyholcomb courtneyholcomb commented Jun 12, 2024

Update SqlWindowFunction enum in a few ways:

  • Add AVERAGE & LAST_VALUE for cumulative metrics with non-default granularities.
  • Remove ROW_NUMBER since it isn't used anywhere.
  • Update values to be uppercase for consistency with our other SQL rendering practices.

All snapshot updates are just upper-casing from first_value to FIRST_VALUE.

@cla-bot cla-bot bot added the cla:yes label Jun 12, 2024
@dbt-labs dbt-labs deleted a comment from github-actions bot Jun 12, 2024
@courtneyholcomb courtneyholcomb marked this pull request as ready for review June 12, 2024 17:55
@courtneyholcomb courtneyholcomb added the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Jun 12, 2024
@courtneyholcomb courtneyholcomb temporarily deployed to DW_INTEGRATION_TESTS June 12, 2024 17:55 — with GitHub Actions Inactive
@courtneyholcomb courtneyholcomb temporarily deployed to DW_INTEGRATION_TESTS June 12, 2024 17:55 — with GitHub Actions Inactive
@dbt-labs dbt-labs deleted a comment from github-actions bot Jun 12, 2024
@courtneyholcomb courtneyholcomb temporarily deployed to DW_INTEGRATION_TESTS June 12, 2024 17:55 — with GitHub Actions Inactive
@courtneyholcomb courtneyholcomb temporarily deployed to DW_INTEGRATION_TESTS June 12, 2024 17:55 — with GitHub Actions Inactive
@github-actions github-actions bot removed the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Jun 12, 2024
Base automatically changed from court/more-defaults to main June 12, 2024 18:49
Add AVERAGE for cumulative metrics with non-default granularities. Remove ROW_NUMBER since it isn't used anywhere. Update values to be uppercase for consistency with our other SQL rendering practices.
FIRST_VALUE = "first_value"
ROW_NUMBER = "row_number"
FIRST_VALUE = "FIRST_VALUE"
LAST_VALUE = "LAST_VALUE"
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to include this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which part? The uppercasing?

Copy link
Contributor

Choose a reason for hiding this comment

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

the last_value enum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes! Sorry forgot to include in the PR description! Will also be used for non-default granularities with cumulative metrics. We could use FIRST_VALUE and order desc, but I decided this was more readable because it aligns with the YAML argument names (which are first/last).

@courtneyholcomb courtneyholcomb enabled auto-merge (squash) June 12, 2024 19:12
@courtneyholcomb courtneyholcomb merged commit 59f40d5 into main Jun 12, 2024
16 checks passed
@courtneyholcomb courtneyholcomb deleted the court/window-funcs branch June 12, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants