Skip to content

Commit

Permalink
Fix issue where BQ refused to generate "complex SQL" for a select (#2061
Browse files Browse the repository at this point in the history
)

* Fix issue where BQ refused to generate "complex SQL" for a select

* Condition this behavior on ##! unsafe_complex_select_query compiler flag

* Add a TODO on the test
  • Loading branch information
christopherswenson authored Dec 19, 2024
1 parent bbf6027 commit eba240c
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 1 deletion.
12 changes: 12 additions & 0 deletions packages/malloy/src/lang/test/expressions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,18 @@ describe('expressions', () => {
errorMessage('Filtered expression requires an aggregate computation')
);
});
test('can use calculate with partition by in select', () => {
expect(markSource`
##! experimental { partition_by function_order_by }
run: a -> {
select: ai, astr
calculate: prev is lag(ai) {
partition_by: astr
order_by: ai asc
}
order_by: ai asc, astr asc
}`).toTranslate();
});

describe('expr props', () => {
test('aggregate order by not allowed without experiments enabled', () => {
Expand Down
17 changes: 16 additions & 1 deletion packages/malloy/src/model/malloy_query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ import {
shouldMaterialize,
} from './materialization/utils';
import {EventStream} from '../runtime_types';
import {Tag} from '../tags';

interface TurtleDefPlus extends TurtleDef, Filtered {}

Expand Down Expand Up @@ -3411,9 +3412,13 @@ class QueryQuery extends QueryField {
};
this.generateStage0Fields(this.rootResult, f, stageWriter);

if (this.firstSegment.type === 'project') {
if (
this.firstSegment.type === 'project' &&
!this.parent.modelCompilerFlags().has('unsafe_complex_select_query')
) {
throw new Error('PROJECT cannot be used on queries with turtles');
}

const groupBy = 'GROUP BY ' + f.dimensionIndexes.join(',') + '\n';

from += this.parent.dialect.sqlGroupSetTable(this.maxGroupSet) + '\n';
Expand Down Expand Up @@ -4312,6 +4317,16 @@ class QueryStruct {
this.addFieldsFromFieldList(structDef.fields);
}

private _modelTag: Tag | undefined = undefined;
modelCompilerFlags(): Tag {
if (this._modelTag === undefined) {
const annotation = this.structDef.modelAnnotation;
const {tag} = Tag.annotationToTag(annotation, {prefix: /^##!\s*/});
this._modelTag = tag;
}
return this._modelTag;
}

protected findFirstDialect(): string {
if (isSourceDef(this.structDef)) {
return this.structDef.dialect;
Expand Down
36 changes: 36 additions & 0 deletions test/src/databases/all/functions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1741,6 +1741,42 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => {
{name: 'UNITED AIR LINES INC', r: 1},
]);
});

// TODO remove the need for the `##! unsafe_complex_select_query` compiler flag
it('can be used in a select', async () => {
await expect(`
##! experimental { function_order_by partition_by }
##! unsafe_complex_select_query
run: state_facts -> {
select: state, births, popular_name
calculate: prev_births_by_name is lag(births) {
partition_by: popular_name
order_by: births desc
}
order_by: births desc
limit: 3
}
`).malloyResultMatches(expressionModel, [
{
state: 'CA',
births: 28810563,
popular_name: 'Isabella',
prev_births_by_name: null,
},
{
state: 'NY',
births: 23694136,
popular_name: 'Isabella',
prev_births_by_name: 28810563,
},
{
state: 'TX',
births: 21467359,
popular_name: 'Isabella',
prev_births_by_name: 23694136,
},
]);
});
});
});

Expand Down

0 comments on commit eba240c

Please sign in to comment.