ensure that only one limit is used on query #1221
Conversation
…d a max_rows_to_read Fixes HDX-2440
🦋 Changeset detectedLatest commit: c90f690 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.
|
| const { width, startResize } = useResizable(16, 'left'); | ||
|
|
||
| const { data: countData } = useExplainQuery(chartConfig); | ||
| const numRows: number = countData?.[0]?.rows ?? 0; |
There was a problem hiding this comment.
Removing unused code
| ...this.getClickHouseSettings(), | ||
| // Max 15 seconds to get keys | ||
| timeout_overflow_mode: 'break', | ||
| max_execution_time: 15, |
There was a problem hiding this comment.
@MikeShi42 you said "shorter query timeout" in the ticket- I guessed 15 seconds. Is this reasonable?
E2E Test Results✅ All tests passed • 25 passed • 3 skipped • 202s
|
| timeout_overflow_mode: 'break', | ||
| max_execution_time: 15, | ||
| // Set the value to 0 (unlimited) so that the LIMIT is used instead | ||
| max_rows_to_read: '0', |
There was a problem hiding this comment.
you can probably set this to undefined instead
There was a problem hiding this comment.
If we set it toundefined then it gets merged with the default settings and gets set to what you have defined in settings. But if it is set, this has a higher priority
There was a problem hiding this comment.
Hmm I was thinking it would overwrite it but you're right. Alright let's give this a shot
|
Tested this change in stagingv2 and everything seemed to work well even on large time ranges (30d) |
Before: Both
limitandmax_rows_readsent with query, which can conflict and result in misses in readsAfter:
limitis sent andmax_rows_readis set to unlimited rows so that thelimitwill always be met. Additionally, set a max query time of 15 seconds andbreakso that after 15 seconds results will always be shown.Fixes HDX-2440