From 3740e17e526c422cc48fd65048dbd2016c99efef Mon Sep 17 00:00:00 2001 From: lloyd tabb Date: Wed, 27 Nov 2024 08:42:42 -0800 Subject: [PATCH] nested pipelines generate bad order by (#2022) * nested pipelines generate bad order by * ordering an reduce is now enforced at translation time * plus, minus, whatever .... * mark default orderings to make lenses work * add tests for default orderings * delete pre 4.0 TOP N BY because it became costly * keep default order by sane * add canOrderB * slightly better order by copmputation * use first, not last, field for default sort * ignore analytic fields in order by --------- Co-authored-by: Michael Toy <66150587+mtoy-googly-moogly@users.noreply.github.com> --- packages/malloy/src/dialect/dialect.ts | 4 +- packages/malloy/src/dialect/duckdb/duckdb.ts | 22 +++- packages/malloy/src/lang/ast/index.ts | 1 - .../lang/ast/query-builders/reduce-builder.ts | 114 ++++++++++++------ .../src/lang/ast/query-properties/top.ts | 97 --------------- .../lang/ast/view-elements/refine-utils.ts | 10 +- .../malloy/src/lang/grammar/MalloyParser.g4 | 2 +- packages/malloy/src/lang/malloy-to-ast.ts | 25 +--- packages/malloy/src/lang/test/query.spec.ts | 107 +++++++++------- packages/malloy/src/model/malloy_query.ts | 22 ++-- packages/malloy/src/model/malloy_types.ts | 17 ++- test/src/databases/all/nomodel.spec.ts | 78 ++++++++++++ 12 files changed, 275 insertions(+), 224 deletions(-) delete mode 100644 packages/malloy/src/lang/ast/query-properties/top.ts diff --git a/packages/malloy/src/dialect/dialect.ts b/packages/malloy/src/dialect/dialect.ts index f70d58f99..166745b47 100644 --- a/packages/malloy/src/dialect/dialect.ts +++ b/packages/malloy/src/dialect/dialect.ts @@ -37,6 +37,7 @@ import { LeafAtomicTypeDef, isRawCast, isLeafAtomic, + OrderBy, } from '../model/malloy_types'; import {DialectFunctionOverloadDef} from './functions'; @@ -237,7 +238,8 @@ export abstract class Dialect { abstract sqlCreateFunctionCombineLastStage( lastStageName: string, - fieldList: DialectFieldList + fieldList: DialectFieldList, + orderBy: OrderBy[] | undefined ): string; abstract sqlCreateTableAsSelect(tableName: string, sql: string): string; diff --git a/packages/malloy/src/dialect/duckdb/duckdb.ts b/packages/malloy/src/dialect/duckdb/duckdb.ts index cd9a722ed..a8f763aa4 100644 --- a/packages/malloy/src/dialect/duckdb/duckdb.ts +++ b/packages/malloy/src/dialect/duckdb/duckdb.ts @@ -32,6 +32,7 @@ import { MeasureTimeExpr, LeafAtomicTypeDef, TD, + OrderBy, } from '../../model/malloy_types'; import {indent} from '../../model/utils'; import { @@ -247,11 +248,28 @@ export class DuckDBDialect extends PostgresBase { sqlCreateFunctionCombineLastStage( lastStageName: string, - dialectFieldList: DialectFieldList + dialectFieldList: DialectFieldList, + orderBy: OrderBy[] | undefined ): string { + let o = ''; + if (orderBy) { + const clauses: string[] = []; + for (const c of orderBy) { + if (typeof c.field === 'string') { + clauses.push(`${c.field} ${c.dir || 'asc'}`); + } else { + clauses.push( + `${dialectFieldList[c.field].sqlOutputName} ${c.dir || 'asc'}` + ); + } + } + if (clauses.length > 0) { + o = ` ORDER BY ${clauses.join(', ')}`; + } + } return `SELECT LIST(STRUCT_PACK(${dialectFieldList .map(d => this.sqlMaybeQuoteIdentifier(d.sqlOutputName)) - .join(',')})) FROM ${lastStageName}\n`; + .join(',')})${o}) FROM ${lastStageName}\n`; } sqlSelectAliasAsStruct( diff --git a/packages/malloy/src/lang/ast/index.ts b/packages/malloy/src/lang/ast/index.ts index 956ef1441..3be05f0b5 100644 --- a/packages/malloy/src/lang/ast/index.ts +++ b/packages/malloy/src/lang/ast/index.ts @@ -115,7 +115,6 @@ export * from './query-properties/ordering'; export * from './query-properties/project-statement'; export * from './query-properties/qop-desc'; export * from './query-properties/sampling'; -export * from './query-properties/top'; export * from './source-elements/named-source'; export * from './source-elements/query-source'; export * from './source-elements/sql-source'; diff --git a/packages/malloy/src/lang/ast/query-builders/reduce-builder.ts b/packages/malloy/src/lang/ast/query-builders/reduce-builder.ts index 2f79a8dc1..6441ef8c9 100644 --- a/packages/malloy/src/lang/ast/query-builders/reduce-builder.ts +++ b/packages/malloy/src/lang/ast/query-builders/reduce-builder.ts @@ -26,18 +26,23 @@ import { FilterCondition, PartialSegment, PipeSegment, + QueryFieldDef, QuerySegment, ReduceSegment, + canOrderBy, + expressionIsAggregate, + expressionIsAnalytic, + hasExpression, isPartialSegment, isQuerySegment, isReduceSegment, + isTemporalField, } from '../../../model/malloy_types'; import {ErrorFactory} from '../error-factory'; -import {SourceFieldSpace} from '../types/field-space'; +import {FieldName, SourceFieldSpace} from '../types/field-space'; import {Limit} from '../query-properties/limit'; import {Ordering} from '../query-properties/ordering'; -import {Top} from '../query-properties/top'; import {QueryProperty} from '../types/query-property'; import {QueryBuilder} from '../types/query-builder'; import { @@ -52,8 +57,15 @@ import { mergeCompositeFieldUsage, } from '../../../model/composite_source_utils'; +function queryFieldName(qf: QueryFieldDef): string { + if (qf.type === 'fieldref') { + return qf.path[qf.path.length - 1]; + } + return qf.name; +} + export abstract class QuerySegmentBuilder implements QueryBuilder { - order?: Top | Ordering; + order?: Ordering; limit?: number; alwaysJoins: string[] = []; abstract inputFS: QueryInputSpace; @@ -68,25 +80,6 @@ export abstract class QuerySegmentBuilder implements QueryBuilder { } if (qp instanceof DefinitionList) { this.resultFS.pushFields(...qp.list); - } else if (qp instanceof Top) { - if (this.limit) { - qp.logError( - 'limit-already-specified', - 'Query operation already limited' - ); - } else { - this.limit = qp.limit; - } - if (qp.by) { - if (this.order) { - qp.logError( - 'ordering-already-specified', - 'Query operation is already sorted' - ); - } else { - this.order = qp; - } - } } else if (qp instanceof Limit) { if (this.limit) { qp.logError( @@ -116,30 +109,21 @@ export abstract class QuerySegmentBuilder implements QueryBuilder { refineFrom(from: PipeSegment | undefined, to: QuerySegment): void { if (from && from.type !== 'index' && from.type !== 'raw') { - if (!this.order) { - if (from.orderBy) { - to.orderBy = from.orderBy; - } else if (from.by) { - to.by = from.by; - } + if (!this.limit && from.orderBy && !from.defaultOrderBy) { + to.orderBy = from.orderBy; } if (!this.limit && from.limit) { to.limit = from.limit; } } - if (this.limit) { - to.limit = this.limit; + if (this.order) { + to.orderBy = this.order.getOrderBy(this.inputFS); + delete to.defaultOrderBy; } - if (this.order instanceof Top) { - const topBy = this.order.getBy(this.inputFS); - if (topBy) { - to.by = topBy; - } - } - if (this.order instanceof Ordering) { - to.orderBy = this.order.getOrderBy(this.inputFS); + if (this.limit) { + to.limit = this.limit; } const oldFilters = from?.filterList || []; @@ -196,6 +180,60 @@ export class ReduceBuilder extends QuerySegmentBuilder implements QueryBuilder { const reduceSegment = this.resultFS.getQuerySegment(from); this.refineFrom(from, reduceSegment); + if (reduceSegment.orderBy) { + // In the modern world, we will ONLY allow names and not numbers in order by lists + for (const by of reduceSegment.orderBy) { + if (typeof by.field === 'number') { + const by_field = reduceSegment.queryFields[by.field - 1]; + by.field = queryFieldName(by_field); + } + } + } + if (reduceSegment.orderBy === undefined || reduceSegment.defaultOrderBy) { + // In the modern world, we will order all reduce segments with the default ordering + let usableDefaultOrderField: string | undefined; + for (const field of reduceSegment.queryFields) { + let fieldAggregate = false; + let fieldAnalytic = false; + let fieldType: string; + const fieldName = queryFieldName(field); + if (field.type === 'fieldref') { + const lookupPath = field.path.map(el => new FieldName(el)); + const refField = this.inputFS.lookup(lookupPath).found; + if (refField) { + const typeDesc = refField.typeDesc(); + fieldType = typeDesc.type; + fieldAggregate = expressionIsAggregate(typeDesc.expressionType); + fieldAnalytic = expressionIsAnalytic(typeDesc.expressionType); + } else { + continue; + } + } else { + fieldType = field.type; + fieldAggregate = + hasExpression(field) && expressionIsAggregate(field.expressionType); + fieldAnalytic = + hasExpression(field) && expressionIsAnalytic(field.expressionType); + } + if (isTemporalField(fieldType) || fieldAggregate) { + reduceSegment.defaultOrderBy = true; + reduceSegment.orderBy = [{field: fieldName, dir: 'desc'}]; + usableDefaultOrderField = undefined; + break; + } + if ( + canOrderBy(fieldType) && + !fieldAnalytic && + !usableDefaultOrderField + ) { + usableDefaultOrderField = fieldName; + } + } + if (usableDefaultOrderField) { + reduceSegment.defaultOrderBy = true; + reduceSegment.orderBy = [{field: usableDefaultOrderField, dir: 'asc'}]; + } + } return reduceSegment; } } diff --git a/packages/malloy/src/lang/ast/query-properties/top.ts b/packages/malloy/src/lang/ast/query-properties/top.ts deleted file mode 100644 index e171e64b3..000000000 --- a/packages/malloy/src/lang/ast/query-properties/top.ts +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * Permission is hereby granted, free of charge, to any person obtaining - * a copy of this software and associated documentation files - * (the "Software"), to deal in the Software without restriction, - * including without limitation the rights to use, copy, modify, merge, - * publish, distribute, sublicense, and/or sell copies of the Software, - * and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be - * included in all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. - * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY - * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, - * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE - * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ - -import { - By as ModelBy, - expressionIsAggregate, - expressionIsAnalytic, -} from '../../../model/malloy_types'; - -import {ExpressionDef} from '../types/expression-def'; -import {FieldName, FieldSpace} from '../types/field-space'; -import {MalloyElement} from '../types/malloy-element'; -import { - LegalRefinementStage, - QueryPropertyInterface, -} from '../types/query-property-interface'; - -type TopInit = FieldName | ExpressionDef; - -export class Top extends MalloyElement implements QueryPropertyInterface { - elementType = 'top'; - queryRefinementStage = LegalRefinementStage.Tail; - forceQueryClass = undefined; - - constructor( - readonly limit: number, - readonly by?: TopInit - ) { - super(); - this.has({by: by}); - } - - getBy(fs: FieldSpace): ModelBy | undefined { - if (this.by) { - if (this.by instanceof FieldName) { - if (fs.isQueryFieldSpace()) { - // TODO jump-to-definition now that we can lookup fields in the output space, - // we need to actually add the reference when we do so. - const output = fs.outputSpace(); - const entry = this.by.getField(output); - if (entry.error) { - this.by.logError(entry.error.code, entry.error.message); - } - if (!entry.found || !entry.isOutputField) { - this.by.logError( - 'top-by-not-found-in-output', - `Unknown field ${this.by.refString} in output space` - ); - } - if (expressionIsAnalytic(entry.found?.typeDesc().expressionType)) { - this.by.logError( - 'top-by-analytic', - `Illegal order by of analytic field ${this.by.refString}` - ); - } - } - return {by: 'name', name: this.by.refString}; - } else { - const byExpr = this.by.getExpression(fs); - if (expressionIsAggregate(byExpr.expressionType)) { - this.by.logError( - 'top-by-aggregate', - 'top by expression must not be an aggregate' - ); - } - if (byExpr.evalSpace === 'output') { - this.by.logError( - 'top-by-not-in-output', - 'top by expression must be an output expression' - ); - } - return {by: 'expression', e: byExpr.value}; - } - } - return undefined; - } -} diff --git a/packages/malloy/src/lang/ast/view-elements/refine-utils.ts b/packages/malloy/src/lang/ast/view-elements/refine-utils.ts index 47929a299..c5a02b3b6 100644 --- a/packages/malloy/src/lang/ast/view-elements/refine-utils.ts +++ b/packages/malloy/src/lang/ast/view-elements/refine-utils.ts @@ -64,13 +64,9 @@ export function refine( } if (from.type !== 'index' && to.type !== 'index' && from.type !== 'raw') { - if (from.orderBy !== undefined || from.by !== undefined) { - if (to.orderBy === undefined && to.by === undefined) { - if (from.orderBy) { - to.orderBy = from.orderBy; - } else if (from.by) { - to.by = from.by; - } + if (from.orderBy !== undefined && !from.defaultOrderBy) { + if (to.orderBy === undefined || to.defaultOrderBy) { + to.orderBy = from.orderBy; } else { logTo.logError( 'ordering-overridden-in-refinement', diff --git a/packages/malloy/src/lang/grammar/MalloyParser.g4 b/packages/malloy/src/lang/grammar/MalloyParser.g4 index 2d5e47ccb..92ca51144 100644 --- a/packages/malloy/src/lang/grammar/MalloyParser.g4 +++ b/packages/malloy/src/lang/grammar/MalloyParser.g4 @@ -448,7 +448,7 @@ bySpec ; topStatement - : TOP INTEGER_LITERAL bySpec? + : TOP INTEGER_LITERAL ; indexElement diff --git a/packages/malloy/src/lang/malloy-to-ast.ts b/packages/malloy/src/lang/malloy-to-ast.ts index a32f4a4de..78ed0f74e 100644 --- a/packages/malloy/src/lang/malloy-to-ast.ts +++ b/packages/malloy/src/lang/malloy-to-ast.ts @@ -1090,30 +1090,9 @@ export class MalloyToAST return this.astAt(new ast.Ordering(orderList), pcx); } - visitTopStatement(pcx: parse.TopStatementContext): ast.Top { - const byCx = pcx.bySpec(); + visitTopStatement(pcx: parse.TopStatementContext): ast.Limit { const topN = this.getNumber(pcx.INTEGER_LITERAL()); - let top: ast.Top | undefined; - if (byCx) { - this.m4advisory( - byCx, - 'top-by', - 'by clause of top statement unupported. Use order_by instead' - ); - const nameCx = byCx.fieldName(); - if (nameCx) { - const name = this.getFieldName(nameCx); - top = new ast.Top(topN, name); - } - const exprCx = byCx.fieldExpr(); - if (exprCx) { - top = new ast.Top(topN, this.getFieldExpr(exprCx)); - } - } - if (!top) { - top = new ast.Top(topN, undefined); - } - return this.astAt(top, pcx); + return this.astAt(new ast.Limit(topN), pcx); } visitTopLevelQueryDefs( diff --git a/packages/malloy/src/lang/test/query.spec.ts b/packages/malloy/src/lang/test/query.spec.ts index a38575fe6..b8f38a5ed 100644 --- a/packages/malloy/src/lang/test/query.spec.ts +++ b/packages/malloy/src/lang/test/query.spec.ts @@ -1018,50 +1018,75 @@ describe('query:', () => { test('top N', () => { expect('run: a->{ top: 5; group_by: astr }').toTranslate(); }); - test('top N by field', () => { - expect( - `##! m4warnings=warn - run: a->{top: 5 ${'by astr'}; group_by: astr}` - ).toLog( - warningMessage( - 'by clause of top statement unupported. Use order_by instead' - ) - ); - }); - test('top N by expression', () => { - expect( - `##! m4warnings=warn - run: ab->{top: 5 by ai + 1; group_by: ai}` - ).toLog( - warningMessage( - 'by clause of top statement unupported. Use order_by instead' - ) - ); - }); test('limit N', () => { expect('run: a->{ limit: 5; group_by: astr }').toTranslate(); }); - test('order by', () => { - expect('run: a->{ order_by: astr; group_by: astr }').toTranslate(); - }); - test('order by preserved over refinement', () => { - expect(` - query: a1 is a -> { group_by: astr } - run: a1 + { order_by: astr } - `).toTranslate(); - }); - test('order by must be in the output space', () => - expect('run: a -> { order_by: af; group_by: astr }').toLog( - errorMessage('Unknown field af in output space') - )); - test('order by asc', () => { - expect('run: a->{ order_by: astr asc; group_by: astr }').toTranslate(); - }); - test('order by desc', () => { - expect('run: a->{ order_by: astr desc; group_by: astr }').toTranslate(); - }); - test('order by N', () => { - expect('run: a->{ order_by: 1 asc; group_by: astr }').toTranslate(); + describe('order by variations', () => { + test('order by', () => { + expect('run: a->{ order_by: astr; group_by: astr }').toTranslate(); + }); + test('order by preserved over refinement', () => { + expect(` + query: a1 is a -> { group_by: astr } + run: a1 + { order_by: astr } + `).toTranslate(); + }); + test('order by must be in the output space', () => + expect('run: a -> { order_by: af; group_by: astr }').toLog( + errorMessage('Unknown field af in output space') + )); + test('order by asc', () => { + expect('run: a->{ order_by: astr asc; group_by: astr }').toTranslate(); + }); + test('order by desc', () => { + expect('run: a->{ order_by: astr desc; group_by: astr }').toTranslate(); + }); + test('order by N', () => { + expect('run: a->{ order_by: 1 asc; group_by: astr }').toTranslate(); + }); + test('first aggregate used for default ordering', () => { + const m = model`run: a->{ + group_by: astr + aggregate: t is ai.sum() + }`; + expect(m).toTranslate(); + const runStmt = m.translator.getQuery(0)!; + expect(runStmt).toBeDefined(); + const reduce = runStmt.pipeline[0]; + expect(reduce.type).toEqual('reduce'); + if (reduce.type === 'reduce') { + expect(reduce.defaultOrderBy).toBeTruthy(); + expect(reduce.orderBy).toEqual([{field: 't', dir: 'desc'}]); + } + }); + test('first temporal used for default ordering', () => { + const m = model`run: a->{ + group_by: astr, ats + }`; + expect(m).toTranslate(); + const runStmt = m.translator.getQuery(0)!; + expect(runStmt).toBeDefined(); + const reduce = runStmt.pipeline[0]; + expect(reduce.type).toEqual('reduce'); + if (reduce.type === 'reduce') { + expect(reduce.defaultOrderBy).toBeTruthy(); + expect(reduce.orderBy).toEqual([{field: 'ats', dir: 'desc'}]); + } + }); + test('first used for ordering when appropriate', () => { + const m = model`run: a->{ + group_by: astr, big is upper(astr) + }`; + expect(m).toTranslate(); + const runStmt = m.translator.getQuery(0)!; + expect(runStmt).toBeDefined(); + const reduce = runStmt.pipeline[0]; + expect(reduce.type).toEqual('reduce'); + if (reduce.type === 'reduce') { + expect(reduce.defaultOrderBy).toBeTruthy(); + expect(reduce.orderBy).toEqual([{field: 'astr', dir: 'asc'}]); + } + }); }); test('order by multiple', () => { expect(` diff --git a/packages/malloy/src/model/malloy_query.ts b/packages/malloy/src/model/malloy_query.ts index f5903ea2b..4324129a5 100644 --- a/packages/malloy/src/model/malloy_query.ts +++ b/packages/malloy/src/model/malloy_query.ts @@ -241,7 +241,8 @@ class StageWriter { } sql += dialect.sqlCreateFunctionCombineLastStage( lastStageName, - getDialectFieldList(structDef) + getDialectFieldList(structDef), + (structDef.resultMetadata as ResultStructMetadataDef)?.orderBy ); const id = `${dialect.udfPrefix}${this.root().udfs.length}`; @@ -332,14 +333,16 @@ class StageWriter { if (!this.useCTE) { return dialect.sqlCreateFunctionCombineLastStage( `(${this.withs[0]})`, - getDialectFieldList(structDef) + getDialectFieldList(structDef), + (structDef.resultMetadata as ResultStructMetadataDef)?.orderBy ); } else { return ( this.combineStages(true).sql + dialect.sqlCreateFunctionCombineLastStage( this.getName(this.withs.length - 1), - getDialectFieldList(structDef) + getDialectFieldList(structDef), + (structDef.resultMetadata as ResultStructMetadataDef)?.orderBy ) ); } @@ -2188,7 +2191,10 @@ class QueryTurtle extends QueryField {} * half translated to the new world of types .. */ export class Segment { - static nextStructDef(structDef: SourceDef, segment: PipeSegment): SourceDef { + static nextStructDef( + structDef: SourceDef, + segment: PipeSegment + ): QueryResultDef { const qs = new QueryStruct( structDef, undefined, @@ -2697,7 +2703,7 @@ class QueryQuery extends QueryField { getResultStructDef( resultStruct: FieldInstanceResult = this.rootResult, isRoot = true - ): SourceDef { + ): QueryResultDef { const fields: FieldDef[] = []; let primaryKey; this.prepare(undefined); @@ -3802,7 +3808,7 @@ class QueryQuery extends QueryField { generateSQLFromPipeline(stageWriter: StageWriter): { lastStageName: string; - outputStruct: SourceDef; + outputStruct: QueryResultDef; } { this.parent.maybeEmitParameterizedSourceUsage(); this.prepare(stageWriter); @@ -4030,11 +4036,11 @@ class QueryQueryRaw extends QueryQuery { // Do nothing! } - getResultStructDef(): SourceDef { + getResultStructDef(): QueryResultDef { if (!isSourceDef(this.parent.structDef)) { throw new Error(`Result cannot by type ${this.parent.structDef.type}`); } - return this.parent.structDef; + return {...this.parent.structDef, type: 'query_result'}; } getResultMetadata( diff --git a/packages/malloy/src/model/malloy_types.ts b/packages/malloy/src/model/malloy_types.ts index 9342391a1..9939742ca 100644 --- a/packages/malloy/src/model/malloy_types.ts +++ b/packages/malloy/src/model/malloy_types.ts @@ -492,10 +492,13 @@ export interface ResultMetadataDef { referenceId?: string; } +export interface Ordered { + orderBy?: OrderBy[]; + defaultOrderBy?: boolean; +} // struct specific metadta -export interface ResultStructMetadataDef extends ResultMetadataDef { +export interface ResultStructMetadataDef extends ResultMetadataDef, Ordered { limit?: number; - orderBy?: OrderBy[]; } export interface ResultMetadata { @@ -630,6 +633,12 @@ export function isAtomicFieldType(s: string): s is AtomicFieldType { 'error', ].includes(s); } +export function canOrderBy(s: string) { + return ['string', 'number', 'date', 'boolean', 'date', 'timestamp'].includes( + s + ); +} + export function isCastType(s: string): s is CastType { return ['string', 'number', 'date', 'timestamp', 'boolean', 'json'].includes( s @@ -1020,13 +1029,11 @@ export interface CompositeFieldUsage { joinedUsage: Record; } -export interface QuerySegment extends Filtered { +export interface QuerySegment extends Filtered, Ordered { type: 'reduce' | 'project' | 'partial'; queryFields: QueryFieldDef[]; extendSource?: FieldDef[]; limit?: number; - by?: By; - orderBy?: OrderBy[]; // uses output field name or index. queryTimezone?: string; alwaysJoins?: string[]; compositeFieldUsage?: CompositeFieldUsage; diff --git a/test/src/databases/all/nomodel.spec.ts b/test/src/databases/all/nomodel.spec.ts index d74ead8de..dedd638d3 100644 --- a/test/src/databases/all/nomodel.spec.ts +++ b/test/src/databases/all/nomodel.spec.ts @@ -1167,6 +1167,84 @@ SELECT row_to_json(finalStage) as row FROM __stage0 AS finalStage`); } ); + test.when( + runtime.supportsNesting && runtime.dialect.supportsPipelinesInViews + )(`Nested pipelines sort properly - ${databaseName}`, async () => { + const result = await runtime + .loadQuery( + ` + source: state_facts is ${databaseName}.table('malloytest.state_facts') + extend { + view: base_view is { + group_by: state + aggregate: airports is sum(airport_count) + order_by: airports asc + } + -> + { + group_by: state + aggregate: airports.sum() + order_by: airports + } + view: base_view2 is { + group_by: state + aggregate: aircrafts is sum(aircraft_count) + order_by: aircrafts asc + } + -> + { + group_by: state + aggregate: aircrafts.sum() + order_by: aircrafts + } + view: base_view3 is { + group_by: state + aggregate: aircrafts is sum(aircraft_count) + } + -> { + group_by: state + aggregate: aircrafts.sum() + } + view: sort_issue is { + where: popular_name ~ r'I' + group_by: popular_name + nest: base_view + nest: base_view2 + nest: base_view3 + } + } + run: state_facts -> sort_issue + ` + ) + .run(); + console.log(result.sql); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const d: any = result.data.toObject(); + const baseView: {state: string; airports: number}[] = d[0]['base_view']; + console.log(baseView); + let baseMax = baseView[0]; + for (const b of baseView) { + expect(b.airports).toBeGreaterThanOrEqual(baseMax.airports); + baseMax = b; + } + + const baseView2: {state: string; aircrafts: number}[] = d[0]['base_view2']; + console.log(baseView2); + let baseMax2 = baseView2[0]; + for (const b of baseView2) { + expect(b.aircrafts).toBeGreaterThanOrEqual(baseMax2.aircrafts); + baseMax2 = b; + } + // implicit order by + const baseView3: {state: string; aircrafts: number}[] = d[0]['base_view3']; + console.log(baseView3); + let baseMax3 = baseView3[0]; + for (const b of baseView3) { + expect(b.aircrafts).toBeLessThanOrEqual(baseMax3.aircrafts); + baseMax3 = b; + } + }); + test.when(runtime.supportsNesting)( 'number as null- ${databaseName}', async () => {