diff --git a/core/src/user-components/tags/table/build-table-sql.ts b/core/src/user-components/tags/table/build-table-sql.ts index ad18471a0b..779ffa64a8 100644 --- a/core/src/user-components/tags/table/build-table-sql.ts +++ b/core/src/user-components/tags/table/build-table-sql.ts @@ -89,6 +89,28 @@ export function buildTableSQLConfig(attrs: TableSQLAttrs): SQLQueryConfig { (c) => c.type === 'measure' && c.processedColumnExpression?.hasAgg && !c.hide ); + // Column-level sorting normally happens after the query so users can re-sort + // without another request. A user limit must instead apply to the sorted set; + // otherwise the warehouse truncates arbitrary groups and the client only sorts + // the surviving rows. Pivoted, comparison, and sparkline render columns still + // require client-side sorting because they do not exist in this query. + const limitSortColumn = + attrs.limit !== undefined && !hasPivots + ? attrs.unifiedColumns.find( + (col) => + col.sort && + col.processedColumnExpression && + col.viz !== 'sparkline' && + col.columnIdForRendering === col.alias && + !col.hide + ) + : undefined; + const order = + attrs.order ?? + (limitSortColumn + ? `${dialect.quoteAlias(limitSortColumn.alias)} ${limitSortColumn.sort}` + : undefined); + const needsSubtotals = Boolean( attrs.subtotals && (hasDimensions || hasPivots) && hasVisibleMeasures && !attrs.limit ); @@ -124,7 +146,7 @@ export function buildTableSQLConfig(attrs: TableSQLAttrs): SQLQueryConfig { date_range: attrs.date_range, having: attrs.having, qualify: attrs.qualify, - order: attrs.order, + order, limit: attrs.limit, page_size: attrs.page_size, offset: attrs.offset, diff --git a/core/src/user-components/tags/table/table.test.ts b/core/src/user-components/tags/table/table.test.ts index 725f9014e9..5dbbd8eb82 100644 --- a/core/src/user-components/tags/table/table.test.ts +++ b/core/src/user-components/tags/table/table.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; import { assertParses, assertRuns, queryClickHouse } from '../../../test-utils/ch-parse'; -import { buildTableSQL, type TableSQLAttrs } from './build-table-sql'; +import { buildTableSQL, buildTableSQLConfig, type TableSQLAttrs } from './build-table-sql'; import { processColumnExpression } from '../../common/sql-expression-utils'; import type { UnifiedColumnDefinition } from './unified-column-definition.types'; import { @@ -940,6 +940,93 @@ describe('table SQL', () => { expect(new FabricDialect().anyValue('x')).toBe('MAX(x)'); }); + it('applies a column sort before limiting grouped rows', () => { + const dialect = new ClickHouseDialect(); + const unifiedColumns = cols(dim('category'), measure('sum(total_sales)'))(dialect); + unifiedColumns[1].sort = 'desc'; + + const { sql } = buildTableSQL({ + data: `( + SELECT * + FROM VALUES( + 'category String, total_sales UInt32', + ('small', 1), + ('largest', 100), + ('medium', 10) + ) + )`, + dataIsSql: true, + unifiedColumns, + limit: 2, + page_size: 10, + dialect + }); + + expect(sql).toMatch(/ORDER BY "sum_total_sales" desc\s+LIMIT 2/); + expect(queryClickHouse(sql).trim().split('\n')).toEqual(['largest\t100', 'medium\t10']); + }); + + it('keeps an explicit table order when a sorted column and limit are both set', () => { + const dialect = new ClickHouseDialect(); + const unifiedColumns = cols(dim('category'), measure('sum(total_sales)'))(dialect); + unifiedColumns[1].sort = 'desc'; + + const { sql } = buildTableSQL({ + data: 'demo.daily_orders', + unifiedColumns, + order: 'category asc', + limit: 10, + page_size: 10, + dialect + }); + + expect(sql).toMatch(/ORDER BY category asc\s+LIMIT 10/); + expect(sql).not.toContain('ORDER BY "sum_total_sales"'); + }); + + it('keeps column sorting client-side when there is no limit', () => { + const dialect = new ClickHouseDialect(); + const unifiedColumns = cols(dim('category'), measure('sum(total_sales)'))(dialect); + unifiedColumns[1].sort = 'desc'; + + const config = buildTableSQLConfig({ + data: 'demo.daily_orders', + unifiedColumns, + dialect + }); + + expect(config.order).toBeUndefined(); + }); + + it('does not push pivoted, sparkline, or derived column sorts into the source query', () => { + const dialect = new ClickHouseDialect(); + const pivotedColumns = cols( + dim('category'), + pivot('date', 'year'), + measure('sum(total_sales)') + )(dialect); + pivotedColumns[2].sort = 'desc'; + + const sparklineColumns = cols(dim('category'), measure('sum(total_sales)'))(dialect); + sparklineColumns[1].sort = 'desc'; + sparklineColumns[1].viz = 'sparkline'; + + const derivedColumns = cols(dim('category'), measure('sum(total_sales)'))(dialect); + derivedColumns[1].sort = 'desc'; + derivedColumns[1].columnIdForRendering = 'sum_total_sales_pct'; + + for (const unifiedColumns of [pivotedColumns, sparklineColumns, derivedColumns]) { + const config = buildTableSQLConfig({ + data: 'demo.daily_orders', + unifiedColumns, + limit: 10, + dialect + }); + + expect(config.order).toBeUndefined(); + } + }); + it('limit disables subtotals even when subtotals=true', () => { const { sql } = buildAllDialects({ data: 'demo.daily_orders',