Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typedef cleanup cleanup #2033

Merged
merged 4 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions packages/malloy-db-trino/src/trino_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,14 @@ import {
SQLSourceDef,
AtomicTypeDef,
mkFieldDef,
isScalarArrayType,
isScalarArray,
RepeatedRecordTypeDef,
RecordTypeDef,
isRepeatedRecord,
Dialect,
ArrayTypeDef,
FieldDef,
TinyParser,
isRepeatedRecordType,
isRepeatedRecord,
} from '@malloydata/malloy';

import {BaseConnection} from '@malloydata/malloy/connection';
Expand Down Expand Up @@ -298,9 +297,9 @@ export abstract class TrinoPrestoConnection
private resultRow(colSchema: AtomicTypeDef, rawRow: unknown) {
if (colSchema.type === 'record') {
return this.convertRow(colSchema.fields, rawRow);
} else if (isRepeatedRecordType(colSchema)) {
} else if (isRepeatedRecord(colSchema)) {
return this.convertNest(colSchema.fields, rawRow) as QueryValue;
} else if (isScalarArrayType(colSchema)) {
} else if (isScalarArray(colSchema)) {
const elType = colSchema.elementTypeDef;
let theArray = this.unpackArray(rawRow);
if (elType.type === 'array') {
Expand Down
11 changes: 4 additions & 7 deletions packages/malloy/src/dialect/snowflake/snowflake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ import {
ArrayLiteralNode,
RecordLiteralNode,
isAtomic,
isRepeatedRecordType,
isScalarArrayType,
isRepeatedRecord,
isScalarArray,
} from '../../model/malloy_types';
import {
DialectFunctionOverloadDef,
Expand Down Expand Up @@ -488,10 +488,7 @@ ${indent(sql)}
} else {
return 'DOUBLE';
}
} else if (
malloyType.type === 'record' ||
isRepeatedRecordType(malloyType)
) {
} else if (malloyType.type === 'record' || isRepeatedRecord(malloyType)) {
const sqlFields = malloyType.fields.reduce((ret, f) => {
if (isAtomic(f)) {
const name = f.as ?? f.name;
Expand All @@ -506,7 +503,7 @@ ${indent(sql)}
return malloyType.type === 'record'
? recordScehma
: `ARRAY(${recordScehma})`;
} else if (isScalarArrayType(malloyType)) {
} else if (isScalarArray(malloyType)) {
return `ARRAY(${this.malloyTypeToSQLType(malloyType.elementTypeDef)})`;
}
return malloyType.type;
Expand Down
17 changes: 8 additions & 9 deletions packages/malloy/src/doc/fielddef.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ interface JoinBase {
}
```

MTOY TODO FIX THE DISCRIMINATORS AGAIN
* `isJoined(fd)` which will return true and grant typed access to the `JoinBase` properties of the `FieldDef`, and because all joined fields are structs, also the `StructDef` properties as well.
* `isJoined(def)` which will return true and grant typed access to the `JoinBase` properties of the object, and because all joined fields are structs, also the `StructDef` properties as well.

## Views

Expand All @@ -92,11 +91,11 @@ are an array of ...
* `join_XXX:` always on query joins


## Descriminators
## Discriminators

* `isTemporalField` -- `date` or `timestamp` type
* `isAtomicFieldType` -- Does the data in this field fit in one column of a table
* `isRepeatedRecord(FieldDef)` -- In some databases this is a type, in other this is an array of record
* `isScalarArray(FieldDef|SrtuctDef)` -- Is a ".each" array
* `isAtomic(FieldDef)` -- Like `isAtomicFieldType` for `FieldDef` instead
* `isLeafAtomic(FieldDef | QueryFieldDef)` -- an Atomic field can be stored in a column, a LeafAtomic is one which isn't a join
* `isTemporalType` -- `date` or `timestamp` type
* `isAtomicFieldType` -- Does type string match the type of one of the atomiv types
* `isRepeatedRecord` -- In some databases this is a type, in other this is an array of record
* `isScalarArray` -- Is a ".each" array
* `isAtomic` -- Like `isAtomicFieldType` for `FieldDef` instead
* `isLeafAtomic` -- an Atomic field can be stored in a column, a LeafAtomic is one which isn't a join
6 changes: 2 additions & 4 deletions packages/malloy/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,16 @@ export type {
ArrayLiteralNode,
} from './model';
export {
isRepeatedRecord,
isSourceDef,
// Used in Composer Demo
Segment,
isLeafAtomic,
isJoinedField,
isJoined,
isJoinedSource,
isSamplingEnable,
isSamplingPercent,
isSamplingRows,
isRepeatedRecordType,
isScalarArrayType,
isRepeatedRecord,
isScalarArray,
mkArrayDef,
mkFieldDef,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import {
AggregateExpr,
Expr,
hasExpression,
isJoinedField,
isAtomic,
isJoined,
} from '../../../model/malloy_types';
import {exprWalk} from '../../../model/utils';

Expand Down Expand Up @@ -267,7 +267,7 @@ function getJoinUsage(fs: FieldSpace, expr: Expr): JoinPath[] {
if (frag.node === 'field') {
const def = lookupWithPath(fs, frag.path);
const field = def.def;
if (isAtomic(field) && !isJoinedField(field)) {
if (isAtomic(field) && !isJoined(field)) {
if (hasExpression(field)) {
const defUsage = getJoinUsage(def.fs, field.e);
result.push(...defUsage.map(r => [...def.joinPath, ...r]));
Expand Down
8 changes: 4 additions & 4 deletions packages/malloy/src/lang/ast/expressions/expr-time-extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import {
ExtractUnit,
isExtractUnit,
isTemporalField,
isTemporalType,
isTimestampUnit,
mkTemporal,
TD,
Expand Down Expand Up @@ -89,13 +89,13 @@ export class ExprTimeExtract extends ExpressionDef {
from: [first, last],
});
}
if (!isTemporalField(first.type)) {
if (!isTemporalType(first.type)) {
return from.first.loggedErrorExpr(
'invalid-type-for-time-extraction',
`Can't extract ${extractTo} from '${first.type}'`
);
}
if (!isTemporalField(last.type)) {
if (!isTemporalType(last.type)) {
return from.last.loggedErrorExpr(
'invalid-type-for-time-extraction',
`Cannot extract ${extractTo} from '${last.type}'`
Expand Down Expand Up @@ -151,7 +151,7 @@ export class ExprTimeExtract extends ExpressionDef {
});
} else {
const argV = from.getExpression(fs);
if (isTemporalField(argV.type)) {
if (isTemporalType(argV.type)) {
return computedExprValue({
dataType: {type: 'number', numberType: 'integer'},
value: {
Expand Down
4 changes: 2 additions & 2 deletions packages/malloy/src/lang/ast/expressions/expr-time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
Expr,
TemporalFieldType,
TypecastExpr,
isTemporalField,
isTemporalType,
} from '../../../model/malloy_types';

import {FieldSpace} from '../types/field-space';
Expand Down Expand Up @@ -58,7 +58,7 @@ export class ExprTime extends ExpressionDef {
dstType: {type: timeType},
e: expr.value,
};
if (isTemporalField(expr.type)) {
if (isTemporalType(expr.type)) {
toTs.srcType = {type: expr.type};
}
value = toTs;
Expand Down
4 changes: 2 additions & 2 deletions packages/malloy/src/lang/ast/expressions/time-literal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {DateTime as LuxonDateTime} from 'luxon';
import {
TemporalFieldType,
TimestampUnit,
isTemporalField,
isTemporalType,
TimeLiteralNode,
} from '../../../model/malloy_types';

Expand Down Expand Up @@ -206,7 +206,7 @@ class GranularLiteral extends TimeLiteral {
}

// Compiler is unsure about rangeEnd = newEnd for some reason
if (rangeEnd && isTemporalField(testValue.type)) {
if (rangeEnd && isTemporalType(testValue.type)) {
const rangeType = testValue.type;
const range = new Range(
new ExprTime(rangeType, rangeStart.value),
Expand Down
13 changes: 5 additions & 8 deletions packages/malloy/src/lang/ast/field-space/static-space.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import {
FieldDef,
StructDef,
SourceDef,
isJoinedField,
isTurtleDef,
isJoined,
isTurtle,
isSourceDef,
JoinFieldDef,
} from '../../../model/malloy_types';
Expand Down Expand Up @@ -71,9 +71,9 @@ export class StaticSpace implements FieldSpace {
}

defToSpaceField(from: FieldDef): SpaceField {
if (isJoinedField(from)) {
if (isJoined(from)) {
return new StructSpaceField(from);
} else if (isTurtleDef(from)) {
} else if (isTurtle(from)) {
return new IRViewField(this, from);
}
return new ColumnSpaceField(from);
Expand Down Expand Up @@ -151,10 +151,7 @@ export class StaticSpace implements FieldSpace {
if (found instanceof SpaceField) {
const definition = found.fieldDef();
if (definition) {
if (
!(found instanceof StructSpaceFieldBase) &&
isJoinedField(definition)
) {
if (!(found instanceof StructSpaceFieldBase) && isJoined(definition)) {
// We have looked up a field which is a join, but not a StructSpaceField
// because it is someting like "dimension: joinedArray is arrayComputation"
// which wasn't known to be a join when the fieldspace was constructed.
Expand Down
4 changes: 2 additions & 2 deletions packages/malloy/src/lang/ast/query-builders/reduce-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
isPartialSegment,
isQuerySegment,
isReduceSegment,
isTemporalField,
isTemporalType,
} from '../../../model/malloy_types';

import {ErrorFactory} from '../error-factory';
Expand Down Expand Up @@ -215,7 +215,7 @@ export class ReduceBuilder extends QuerySegmentBuilder implements QueryBuilder {
fieldAnalytic =
hasExpression(field) && expressionIsAnalytic(field.expressionType);
}
if (isTemporalField(fieldType) || fieldAggregate) {
if (isTemporalType(fieldType) || fieldAggregate) {
reduceSegment.defaultOrderBy = true;
reduceSegment.orderBy = [{field: fieldName, dir: 'desc'}];
usableDefaultOrderField = undefined;
Expand Down
4 changes: 2 additions & 2 deletions packages/malloy/src/lang/ast/typedesc-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
expressionIsScalar,
ExpressionType,
ExpressionValueType,
isRepeatedRecordType,
isRepeatedRecord,
TD,
TypeDesc,
} from '../../model';
Expand Down Expand Up @@ -145,7 +145,7 @@ export function atomicDef(td: AtomicTypeDef | TypeDesc): AtomicTypeDef {
if (TD.isAtomic(td)) {
switch (td.type) {
case 'array': {
return isRepeatedRecordType(td)
return isRepeatedRecord(td)
? {
type: 'array',
elementTypeDef: td.elementTypeDef,
Expand Down
10 changes: 5 additions & 5 deletions packages/malloy/src/lang/ast/types/expression-def.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
Expr,
TimestampUnit,
isDateUnit,
isTemporalField,
isTemporalType,
expressionIsAggregate,
TD,
LeafExpressionType,
Expand Down Expand Up @@ -181,7 +181,7 @@ export class ExprDuration extends ExpressionDef {
): ExprValue {
const lhs = left.getExpression(fs);
this.typeCheck(this, lhs);
if (isTemporalField(lhs.type) && (op === '+' || op === '-')) {
if (isTemporalType(lhs.type) && (op === '+' || op === '-')) {
const num = this.n.getExpression(fs);
if (!TDU.typeEq(num, TDU.numberT)) {
this.logError(
Expand Down Expand Up @@ -253,8 +253,8 @@ function timeCompare(
op: CompareMalloyOperator,
rhs: ExprValue
): Expr | undefined {
const leftIsTime = isTemporalField(lhs.type);
const rightIsTime = isTemporalField(rhs.type);
const leftIsTime = isTemporalType(lhs.type);
const rightIsTime = isTemporalType(rhs.type);
const node = getExprNode(op);
if (leftIsTime && rightIsTime) {
if (lhs.type !== rhs.type) {
Expand Down Expand Up @@ -459,7 +459,7 @@ function delta(
return noGo;
}

const timeLHS = isTemporalField(lhs.type);
const timeLHS = isTemporalType(lhs.type);

const err = errorCascade(timeLHS ? 'error' : 'number', lhs, rhs);
if (err) return err;
Expand Down
4 changes: 2 additions & 2 deletions packages/malloy/src/lang/ast/view-elements/reference-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
PipeSegment,
SourceDef,
isAtomic,
isTurtleDef,
isTurtle,
sourceBase,
} from '../../../model/malloy_types';
import {ErrorFactory} from '../error-factory';
Expand Down Expand Up @@ -107,7 +107,7 @@ export class ReferenceView extends View {
name,
outputStruct,
};
} else if (isTurtleDef(fieldDef)) {
} else if (isTurtle(fieldDef)) {
if (this.reference.list.length > 1) {
if (forRefinement) {
this.logError(
Expand Down
2 changes: 1 addition & 1 deletion packages/malloy/src/lang/test/field-symbols.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe('structdef comprehension', () => {
});

test('import repeated record', () => {
const field: model.LeafArrayDef = {
const field: model.ScalarArrayDef = {
name: 't',
type: 'array',
dialect: 'standardsql',
Expand Down
6 changes: 3 additions & 3 deletions packages/malloy/src/lang/test/imports.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
import {isJoinedField} from '../../model';
import {isJoined} from '../../model';
import './parse-expects';
import {TestTranslator, errorMessage, model} from './test-translator';
import escapeRegEx from 'lodash/escapeRegExp';
Expand Down Expand Up @@ -91,8 +91,8 @@ source: botProjQSrc is botProjQ
const maybeField = newSrc?.fields.find(f => f.name === 'b');
expect(maybeField).toBeDefined();
const f = maybeField!;
expect(isJoinedField(f)).toBeTruthy();
if (isJoinedField(f)) {
expect(isJoined(f)).toBeTruthy();
if (isJoined(f)) {
expect(f.type).toBe('query_source');
if (f.type === 'query_source') {
expect(typeof f.query.structRef).not.toBe('string');
Expand Down
4 changes: 2 additions & 2 deletions packages/malloy/src/lang/test/query.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
QueryFieldDef,
QuerySegment,
expressionIsCalculation,
isJoinedField,
isJoined,
isQuerySegment,
isAtomic,
} from '../../model';
Expand Down Expand Up @@ -1327,7 +1327,7 @@ describe('query:', () => {
const q = t.translated.queryList[0].pipeline[0];
if (q.type === 'reduce' && q.extendSource) {
expect(q.extendSource.length).toBe(1);
expect(isJoinedField(q.extendSource[0])).toBeTruthy();
expect(isJoined(q.extendSource[0])).toBeTruthy();
expect(q.extendSource[0].type).toBe('table');
} else {
fail('Did not generate extendSource');
Expand Down
3 changes: 1 addition & 2 deletions packages/malloy/src/malloy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ import {
SourceDef,
isSourceDef,
QueryToMaterialize,
isJoinedField,
isJoined,
} from './model';
import {
Expand Down Expand Up @@ -1573,7 +1572,7 @@ export class Explore extends Entity implements Taggable {
this.structDef.fields.map(fieldDef => {
const name = fieldDef.as || fieldDef.name;
const sourceField = sourceFields.get(fieldDef.name);
if (isJoinedField(fieldDef)) {
if (isJoined(fieldDef)) {
return [name, new ExploreField(fieldDef, this, sourceField)];
} else if (fieldDef.type === 'turtle') {
return [name, new QueryField(fieldDef, this, sourceField)];
Expand Down
Loading
Loading