fix(common-utils): rank text-index Map keys by frequency

.changeset/text-index-key-order.md

@@ -0,0 +1,13 @@
+---
+'@hyperdx/common-utils': patch
+'@hyperdx/app': patch
+'@hyperdx/api': patch
+---
+
+fix: rank Map keys by frequency when a text index serves key discovery
+
+The two `mergeTreeTextIndex` paths in `getMapKeys` grouped keys and applied
+`LIMIT maxKeys` with no `ORDER BY`, so on a column with more keys than the
+limit the filter sidebar showed an arbitrary subset. They now order by
+`sum(cardinality) DESC, key`, which keeps the most frequent keys, as the key
+rollup path already does with `sum(count) DESC`.

packages/common-utils/src/__tests__/metadata.int.test.ts

@@ -1411,4 +1411,106 @@ describe('Metadata Integration Tests', () => {
       expect(keys).toEqual(['legacy.key']);
     });
   });
+
+  describe.each([
+    {
+      path: 'key',
+      tableName: 'test_map_keys_key_text_index_order',
+      indexDefinition: `INDEX idx_log_attr_key mapKeys(LogAttributes) TYPE text(tokenizer = 'array') GRANULARITY 1`,
+    },
+    {
+      path: 'kv',
+      tableName: 'test_map_keys_kv_text_index_order',
+      indexDefinition: `LogAttributeItems Array(String) MATERIALIZED
+          arrayMap(x -> concat(x.1, '=', x.2), CAST(LogAttributes, 'Array(Tuple(String, String))')),
+        INDEX idx_log_attr_items LogAttributeItems TYPE text(tokenizer = 'array') GRANULARITY 1`,
+    },
+  ])(
+    'getMapKeys - $path text index ordering',
+    ({ tableName, indexDefinition }) => {
+      let metadata: Metadata;
+      let textIndexSupported = false;
+
+      beforeAll(async () => {
+        const probe = new Metadata(hdxClient, new MetadataCache());
+        textIndexSupported = supportsMergeTreeTextIndex(
+          await probe.getServerVersion({ connectionId: 'test_connection' }),
+        );
+        if (!textIndexSupported) return;
+
+        await client.command({
+          query: `CREATE OR REPLACE TABLE default.${tableName} (
+              Timestamp DateTime64(9),
+              LogAttributes Map(LowCardinality(String), String),
+              ${indexDefinition}
+            )
+            ENGINE = MergeTree()
+            PARTITION BY toDate(Timestamp)
+            ORDER BY Timestamp
+          `,
+        });
+
+        // The text index dictionary is sorted, so without an ORDER BY the
+        // LIMIT keeps the alphabetically first keys. The frequent keys sort
+        // after the 200 one-off keys, and in reverse order of frequency.
+        const insert = (attributes: string, rows: number) =>
+          client.command({
+            query: `INSERT INTO default.${tableName} (Timestamp, LogAttributes)
+              SELECT now64(9) - INTERVAL 10 MINUTE, ${attributes}
+              FROM numbers(${rows})`,
+          });
+        await insert(`map(concat('rare.', toString(number)), 'v')`, 200);
+        await insert(
+          `map('user.id', 'x', 'trace.flags', 'x', 'service.name', 'x')`,
+          40,
+        );
+        await insert(`map('user.id', 'x', 'trace.flags', 'x')`, 10);
+        await insert(`map('user.id', 'x')`, 10);
+      });
+
+      afterAll(async () => {
+        await client.command({
+          query: `DROP TABLE IF EXISTS default.${tableName}`,
+        });
+      });
+
+      beforeEach(() => {
+        metadata = new Metadata(hdxClient, new MetadataCache());
+      });
+
+      afterEach(() => {
+        jest.restoreAllMocks();
+      });
+
+      it('keeps the most frequent keys when maxKeys truncates the list', async () => {
+        if (!textIndexSupported) {
+          console.warn(
+            'Skipping: ClickHouse < 26.3 does not support mergeTreeTextIndex()',
+          );
+          return;
+        }
+        const querySpy = jest.spyOn(hdxClient, 'query');
+
+        const keys = await metadata.getMapKeys({
+          databaseName: 'default',
+          tableName,
+          column: 'LogAttributes',
+          connectionId: 'test_connection',
+          maxKeys: 3,
+          timestampValueExpression: 'Timestamp',
+          dateRange: [
+            new Date(Date.now() - 60 * 60 * 1000),
+            new Date(Date.now() + 60 * 1000),
+          ],
+        });
+
+        expect(
+          querySpy.mock.calls.some(([{ query }]) =>
+            query.includes('mergeTreeTextIndex'),
+          ),
+        ).toBe(true);
+        expect(keys).toEqual(['user.id', 'trace.flags', 'service.name']);
+      });
+    },
+  );
 });

packages/common-utils/src/__tests__/metadata.test.ts

@@ -2734,6 +2734,84 @@ describe('Metadata', () => {
     });
   });
 
+  describe('getMapKeys (text index paths)', () => {
+    const textIndexLookups: [string, TextIndexInfoLookup][] = [
+      [
+        'key',
+        new Map([
+          [
+            'LogAttributes',
+            {
+              key: {
+                indexName: 'idx_log_attr_keys',
+                mapColumn: 'LogAttributes',
+              },
+            },
+          ],
+        ]),
+      ],
+      [
+        'kv',
+        new Map([
+          [
+            'LogAttributes',
+            {
+              kv: {
+                columnName: 'LogAttributeItems',
+                indexName: 'idx_log_attr_items',
+                separator: '=',
+                useHasAny: true,
+                mapColumn: 'LogAttributes',
+              },
+            },
+          ],
+        ]),
+      ],
+    ];
+
+    afterEach(() => {
+      jest.restoreAllMocks();
+      (mockClickhouseClient.query as jest.Mock).mockReset();
+    });
+
+    it.each(textIndexLookups)(
+      'ranks keys by row count before the LIMIT on the %s text index path',
+      async (_, textIndexLookup) => {
+        const md = new Metadata(mockClickhouseClient, new MetadataCache());
+        jest.spyOn(md, 'getServerVersion').mockResolvedValue([26, 3, 0, 0]);
+        jest
+          .spyOn(md, 'getMapColumnTextIndexes')
+          .mockResolvedValue(textIndexLookup);
+        (mockClickhouseClient.query as jest.Mock).mockResolvedValue({
+          json: () =>
+            Promise.resolve({ data: [{ key: 'http.method' }, { key: 'k8s' }] }),
+        });
+
+        const keys = await md.getMapKeys({
+          databaseName: 'default',
+          tableName: 'otel_logs',
+          column: 'LogAttributes',
+          connectionId: 'test_connection',
+          maxKeys: 2,
+          dateRange: [
+            new Date('2024-01-01T00:00:00Z'),
+            new Date('2024-01-01T01:00:00Z'),
+          ],
+          timestampValueExpression: 'Timestamp',
+        });
+
+        expect(keys).toEqual(['http.method', 'k8s']);
+        expect(mockClickhouseClient.query).toHaveBeenCalledTimes(1);
+        const { query } = (mockClickhouseClient.query as jest.Mock).mock
+          .calls[0][0];
+        expect(query).toContain('FROM mergeTreeTextIndex(');
+        expect(query).toMatch(
+          /GROUP BY key HAVING key != ''\s+ORDER BY sum\(cardinality\) DESC, key\s+LIMIT/,
+        );
+      },
+    );
+  });
+
   describe('getMapValues', () => {
     const buildMetadata = () => {
       const realCache = new (

packages/common-utils/src/core/metadata.ts

@@ -1017,11 +1017,13 @@ export class Metadata {
         timestampValueExpression,
       });
       const index = textIndexInfo.key.indexName;
+      // `cardinality` is the token's row count in the part, not a distinct count.
       const sql = chSql`
         SELECT token AS key
         FROM mergeTreeTextIndex(${{ String: databaseName }}, ${{ String: tableName }}, ${{ String: index }})
         WHERE ${partsFilter}
         GROUP BY key HAVING key != ''
+        ORDER BY sum(cardinality) DESC, key
         LIMIT ${{ Int32: maxKeys }}`;
       try {
         const keys = await this.clickhouseClient
@@ -1063,11 +1065,13 @@ export class Metadata {
       });
       const index = textIndexInfo.kv.indexName;
       const separator = textIndexInfo.kv.separator;
+      // `cardinality` is the token's row count in the part, not a distinct count.
       const sql = chSql`
         SELECT splitByString(${{ String: separator }}, token)[1] AS key
         FROM mergeTreeTextIndex(${{ String: databaseName }}, ${{ String: tableName }}, ${{ String: index }})
         WHERE ${partsFilter}
         GROUP BY key HAVING key != ''
+        ORDER BY sum(cardinality) DESC, key
         LIMIT ${{ Int32: maxKeys }}`;
       try {
         const keys = await this.clickhouseClient