-
Notifications
You must be signed in to change notification settings - Fork 303
feat: enable filters for json columns #1087
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
Conversation
🦋 Changeset detectedLatest commit: 4b77c9e The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
SELECT DISTINCT JSONDynamicPathsWithTypes(${{ Identifier: column }}) as paths | ||
FROM ${tableExpr({ database: databaseName, table: tableName })} ${where} | ||
LIMIT ${{ Int32: maxKeys }} | ||
SETTINGS timeout_overflow_mode = 'break', max_execution_time = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to hardcode this max_execution_time? Where did the 2 come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Came from a suggestion by Mike, I think it's reasonable. It should find all of the common JSON paths + some dynamic paths. But there is an alternate approach using distinctJSONPathsAndTypes
, but is a full aggregation. I think we should try this and gather feedback from large volume users of the JSON type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@knudtty left some feedback. Also the CI tests need some love :)
Stably Runner - Test Suite - 'Smoke Test'Test Suite Run Result: 🟢 Success (4/4 tests passed) [dashboard] This comment was generated from stably-runner-action |
Closes HDX-1965