perf: add prelimit CTE to getMapKeys query + store clickhouse settings in shared cache #1187
Conversation
I believe the reason Anthropic was hitting the memory error is that the `metadataMaxRowsToRead` setting was applied only to a specific query (and did not include the getKeyValues method). Now, since ClickHouse settings are cached and shared across all metadata instances, every method within the Metadata class should respect these settings. This PR also adds another CTE filtering on top of queries that touch array aggregation functions like `groupUniqArray` and `groupUniqArrayArray` I will port this fix to OSS later Ref: HDX-2321
🦋 Changeset detectedLatest commit: dad982a 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.
|
Stably Runner - Test Suite - 'Smoke Test'Test Suite Run Result: 🔴 Failure (1/4 tests failed) [dashboard] Failed Tests: This comment was generated from stably-runner-action |
wrn14897
requested review from
a team and
SpencerTorres
and removed request for
a team and
SpencerTorres
| ...chartConfig, | ||
| with: [ | ||
| { | ||
| name: 'sampledData', |
There was a problem hiding this comment.
This actually introduces the regression noted in HDX-2440. Before we merge this PR we should have a fix prepared to land on top so we can smoothly fix the regression without too much weirdness in downstream git history while not breaking the next build.
There was a problem hiding this comment.
Sounds good. I will take a look at it
Optimize the query performance of the getMapKeys method to prevent excessive resource usage in ClickHouse, even when max_rows_to_read is specified.
Ref: HDX-2411