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: DO-3494: Strip Table column prefix when filtering and sorting #355

Merged
merged 12 commits into from
Aug 6, 2024

Conversation

Roman-Kornev
Copy link
Contributor

Motivation and Context

Due to the fact that Table column conversion happens as the last step, all filtering and sorting queries would try to operate on the raw data using the internal column names.

Implementation Description

Rather than changing the API and forcing all transformations to happen on the converted data, the prefix is stripped altogether. The only downside is potentially in the future when we add automatic column filters: when trying to filter/sort non-unique column names - in that case the filtering would apply to all matching column names instead of the requested column. Right now it is not possible to specify column filters for duplicate column names anyway.

Any new dependencies Introduced

How Has This Been Tested?

Added a pagination ordering test, added more testcases for filtering.

PR Checklist:

  • I have implemented all requirements? (see JIRA, project documentation).
  • I am not affecting someone else's work, If I am, they are included as a reviewer.
  • I have added relevant tests (unit, integration or regression).
  • I have added comments to all the bits that are hard to follow.
  • I have added/updated Documentation.
  • I have updated the appropriate changelog with a line for my changes.

Screenshots (if appropriate):

Copy link
Collaborator

@krzysztof-causalens krzysztof-causalens left a comment

Choose a reason for hiding this comment

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

Nice find, should be good enough for now

@Roman-Kornev Roman-Kornev merged commit 06e5cd0 into master Aug 6, 2024
2 checks passed
@Roman-Kornev Roman-Kornev deleted the table-qs branch August 6, 2024 14:56
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.

2 participants