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.

Sign up for GitHub

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

Jump to bottom

Case insensitivity for query params #802

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Case insensitivity for query params #802

merged 2 commits into from
Oct 10, 2023

Conversation

courtneyholcomb
Copy link
Contributor

@courtneyholcomb courtneyholcomb commented Oct 10, 2023 •
edited
Loading

Description

Support case-insensitive params in group_by and order_by parameters.
A community member recently ran into a bug where they uppercased a dimension in the group_by, which worked, but then the same syntax didn't work in the order_by. This should fix that issue.

@cla-bot cla-bot bot added the cla:yes label Oct 10, 2023
@github-actions
Copy link

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@courtneyholcomb courtneyholcomb marked this pull request as ready for review October 10, 2023 21:44
tlento
tlento approved these changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@tlento tlento left a comment

Choose a reason for hiding this comment

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

Oh interesting, yeah, we require all names to be lower-case so we shouldn't care much what casing gets put in at query time.

What happens with the object syntax though? Is that case-insensitive?

@courtneyholcomb
Copy link
Contributor Author

Oh interesting, yeah, we require all names to be lower-case so we shouldn't care much what casing gets put in at query time.

What happens with the object syntax though? Is that case-insensitive?

@tlento this PR makes the object syntax case-insensitive!

@courtneyholcomb courtneyholcomb merged commit 81ae014 into main Oct 10, 2023
9 checks passed
@courtneyholcomb courtneyholcomb deleted the court/case-insensitivity branch October 10, 2023 23:46
sarbmeetka pushed a commit to sarbmeetka/metricflow that referenced this pull request Nov 13, 2023
@courtneyholcomb @sarbmeetka
Case insensitivity for query params (dbt-labs#802)
e58ec2c 
* Case insensitivity for query params

* Changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DevonFulcher DevonFulcher DevonFulcher approved these changes

@tlento tlento tlento approved these changes

@plypaul plypaul Awaiting requested review from plypaul

Assignees
No one assigned
Labels
cla:yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@courtneyholcomb @tlento @DevonFulcher