Skip to content

Commit

Permalink
Don't compute join usage in computedExprValue
Browse files Browse the repository at this point in the history
  • Loading branch information
christopherswenson committed Oct 22, 2024
1 parent d2570d3 commit c1e394f
Show file tree
Hide file tree
Showing 11 changed files with 23 additions and 40 deletions.
1 change: 0 additions & 1 deletion packages/malloy/src/lang/ast/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,5 @@ export function errorFor(reason: string): ExprValue {
expressionType: 'scalar',
value: {node: 'error', message: reason},
evalSpace: 'constant',
joinUsage: [],
};
}
2 changes: 1 addition & 1 deletion packages/malloy/src/lang/ast/expressions/boolean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ export class Boolean extends ExpressionDef {
}

getExpression(): ExprValue {
return {...FT.boolT, value: {node: this.value}, joinUsage: []};
return {...FT.boolT, value: {node: this.value}};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ export abstract class ExprAggregateFunction extends ExpressionDef {
? {node: 'outputField', name: this.source.refString}
: {node: 'field', path: this.source.path},
evalSpace: footType.evalSpace,
joinUsage: [result.joinPath],
};
structPath = this.source.path;
// Here we handle a special case where you write `foo.agg()` and `foo` is a
Expand Down Expand Up @@ -188,8 +187,6 @@ export abstract class ExprAggregateFunction extends ExpressionDef {
expressionType: 'aggregate',
value: f,
evalSpace: 'output',
// TODO the join usage doesn't matter since you cannot reaggregate aggregates
joinUsage: [],
};
}
return errorFor('aggregate type check');
Expand Down
2 changes: 0 additions & 2 deletions packages/malloy/src/lang/ast/expressions/expr-count.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ export class ExprCount extends ExprAggregateFunction {
expressionType: 'aggregate',
value: ret,
evalSpace: 'output',
// TODO this doesn't actually matter, since you cannot reaggregate an aggregate
joinUsage: [],
};
}
}
3 changes: 0 additions & 3 deletions packages/malloy/src/lang/ast/expressions/expr-func.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export class ExprFunc extends ExpressionDef {
expressionType: footType.expressionType,
value: {node: 'field', path: this.source.path},
evalSpace: footType.evalSpace,
joinUsage: [lookup.joinPath],
};
structPath = this.source.path.slice(0, -1);
} else {
Expand Down Expand Up @@ -424,15 +423,13 @@ export class ExprFunc extends ExpressionDef {
: expressionIsScalar(expressionType)
? maxEvalSpace
: 'output';
const joinUsage = argExprs.flatMap(e => e.joinUsage);
// TODO consider if I can use `computedExprValue` here...
// seems like the rules for the evalSpace is a bit different from normal though
return {
dataType: type.dataType,
expressionType,
value: funcCall,
evalSpace,
joinUsage,
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,23 +41,21 @@ export class ExprIdReference extends ExpressionDef {
getExpression(fs: FieldSpace): ExprValue {
const def = this.fieldReference.getField(fs);
if (def.found) {
const joinUsage = [def.joinPath];
const td = def.found.typeDesc();
if (def.isOutputField) {
return {
...td,
// TODO what about literal??
evalSpace: td.evalSpace === 'constant' ? 'constant' : 'output',
value: {node: 'outputField', name: this.refString},
joinUsage,
};
}
const value = {node: def.found.refType, path: this.fieldReference.path};
// We think that aggregates are more 'output' like, but maybe we will reconsider that...
const evalSpace = expressionIsAggregate(td.expressionType)
? 'output'
: td.evalSpace;
return {...td, value, evalSpace, joinUsage};
return {...td, value, evalSpace};
}
return this.loggedErrorExpr(def.error.code, def.error.message);
}
Expand Down
1 change: 0 additions & 1 deletion packages/malloy/src/lang/ast/expressions/expr-now.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export class ExprNow extends ExpressionDef {
// `now` is considered to be a constant, at least in the dialects we support today
evalSpace: 'constant',
value: {node: 'now'},
joinUsage: [],
};
}
}
2 changes: 0 additions & 2 deletions packages/malloy/src/lang/ast/expressions/expr-ungroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ export class ExprUngroup extends ExpressionDef {
expressionType: 'ungrouped_aggregate',
value: ungroup,
evalSpace: 'output',
// TODO unsure?
joinUsage: exprVal.joinUsage,
};
}
return this.loggedErrorExpr(
Expand Down
24 changes: 3 additions & 21 deletions packages/malloy/src/lang/ast/expressions/time-literal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
TimeLiteralNode,
} from '../../../model/malloy_types';

import {ExprValue} from '../types/expr-value';
import {ExprValue, literalTimeResult} from '../types/expr-value';
import {FieldSpace} from '../types/field-space';
import {Range} from './range';
import {ExprTime} from './expr-time';
Expand Down Expand Up @@ -104,26 +104,8 @@ abstract class TimeLiteral extends ExpressionDef {
}

protected makeValue(val: string, dataType: TemporalFieldType): TimeResult {
const timeFrag = this.makeLiteral(val, dataType);
const expressionType = 'scalar';
const value = timeFrag;
if (this.units) {
return {
dataType,
expressionType,
value,
timeframe: this.units,
evalSpace: 'literal',
joinUsage: [],
};
}
return {
dataType,
expressionType,
value,
evalSpace: 'literal',
joinUsage: [],
};
const value = this.makeLiteral(val, dataType);
return literalTimeResult({value, dataType, timeframe: this.units});
}

getExpression(_fs: FieldSpace): ExprValue {
Expand Down
2 changes: 0 additions & 2 deletions packages/malloy/src/lang/ast/types/expr-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@
*/

import {Expr, TypeDesc} from '../../../model/malloy_types';
import {JoinPath} from './lookup-result';

type MorphicValues = Record<string, Expr>;
export interface ExprResult extends TypeDesc {
value: Expr;
morphic?: MorphicValues;
joinUsage: JoinPath[];
}
19 changes: 18 additions & 1 deletion packages/malloy/src/lang/ast/types/expr-value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export function computedExprValue({
rawType,
expressionType: maxOfExpressionTypes(from.map(e => e.expressionType)),
evalSpace: mergeEvalSpaces(...from.map(e => e.evalSpace)),
joinUsage: from.flatMap(e => e.joinUsage),
};
}

Expand Down Expand Up @@ -98,3 +97,21 @@ export function literalExprValue(options: {
// Makes literal, output, scalar because from is empty
return computedExprValue({...options, from: []});
}

export function literalTimeResult({
value,
dataType,
rawType,
timeframe,
}: {
value: Expr;
rawType?: string;
dataType: TemporalFieldType;
timeframe?: TimestampUnit;
}): TimeResult {
return {
...computedExprValue({value, dataType, rawType, from: []}),
dataType,
timeframe,
};
}

0 comments on commit c1e394f

Please sign in to comment.