perf: disable CTE if disableRowLimit flag is true (getKeyValues method) (#1205)

To pull things like metric tags, `getKeyValues` method should fallback to the query w/o using CTE for better performance

Ref: HDX-2482
This commit is contained in:
Warren 2025-09-25 06:30:21 -07:00 committed by GitHub
parent 24314a9605
commit 4ff55c0e44
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 52 additions and 37 deletions

View file

@ -0,0 +1,5 @@
---
"@hyperdx/common-utils": patch
---
perf: disable CTE if disableRowLimit flag is true (getKeyValues method)

View file

@ -584,46 +584,56 @@ export class Metadata {
return this.cache.getOrFetch(
`${chartConfig.from.databaseName}.${chartConfig.from.tableName}.${keys.join(',')}.${chartConfig.dateRange.toString()}.${disableRowLimit}.values`,
async () => {
// Get all columns including materialized ones
const columns = await this.getColumns({
databaseName: chartConfig.from.databaseName,
tableName: chartConfig.from.tableName,
connectionId: chartConfig.connection,
});
const selectClause = keys
.map((k, i) => `groupUniqArray(${limit})(${k}) AS param${i}`)
.join(', ');
// Build select expression that includes all columns by name
// This ensures materialized columns are included
const selectExpr =
columns.map(col => `\`${col.name}\``).join(', ') || '*';
// When disableRowLimit is true, query directly without CTE
// Otherwise, use CTE with row limits for sampling
const sqlConfig = disableRowLimit
? {
...chartConfig,
select: selectClause,
}
: await (async () => {
// Get all columns including materialized ones
const columns = await this.getColumns({
databaseName: chartConfig.from.databaseName,
tableName: chartConfig.from.tableName,
connectionId: chartConfig.connection,
});
const sql = await renderChartConfig(
{
with: [
{
name: 'sampledData',
chartConfig: {
...chartConfig,
select: selectExpr,
limit: {
limit: !disableRowLimit
? this.getClickHouseSettings().max_rows_to_read
? Number(this.getClickHouseSettings().max_rows_to_read)
: DEFAULT_METADATA_MAX_ROWS_TO_READ
: undefined,
// Build select expression that includes all columns by name
// This ensures materialized columns are included
const selectExpr =
columns.map(col => `\`${col.name}\``).join(', ') || '*';
return {
with: [
{
name: 'sampledData',
chartConfig: {
...chartConfig,
select: selectExpr,
limit: {
limit: this.getClickHouseSettings().max_rows_to_read
? Number(
this.getClickHouseSettings().max_rows_to_read,
)
: DEFAULT_METADATA_MAX_ROWS_TO_READ,
},
},
isSubquery: true,
},
},
isSubquery: true,
},
],
select: keys
.map((k, i) => `groupUniqArray(${limit})(${k}) AS param${i}`)
.join(', '),
connection: chartConfig.connection,
from: { databaseName: '', tableName: 'sampledData' },
where: '',
},
this,
);
],
select: selectClause,
connection: chartConfig.connection,
from: { databaseName: '', tableName: 'sampledData' },
where: '',
};
})();
const sql = await renderChartConfig(sqlConfig, this);
const json = await this.clickhouseClient
.query<'JSON'>({