From 9cd1f697e775e634a322e77b487febead875e90f Mon Sep 17 00:00:00 2001 From: Michael Toy <66150587+mtoy-googly-moogly@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:04:21 -0800 Subject: [PATCH] refactor TypeDef for arrays and records (#2032) * refactor TypeDef for arrays and records * fix trino test * fix join parsing * fix join dscriminators * fixed array of array * fix composite source * ubreak postgres array? --- .../src/bigquery_connection.ts | 11 +- packages/malloy-db-duckdb/src/duckdb.spec.ts | 13 +- .../malloy-db-duckdb/src/duckdb_common.ts | 12 +- .../src/postgres_connection.ts | 11 +- .../src/snowflake_connection.ts | 51 ++++--- .../src/trino_connection.spec.ts | 19 +-- .../malloy-db-trino/src/trino_connection.ts | 74 ++++----- .../malloy/src/dialect/snowflake/snowflake.ts | 10 +- packages/malloy/src/doc/fielddef.md | 1 + packages/malloy/src/index.ts | 8 +- .../expressions/expr-aggregate-function.ts | 4 +- .../ast/expressions/expr-array-literal.ts | 32 ++-- .../ast/expressions/expr-record-literal.ts | 9 +- .../lang/ast/field-space/reference-field.ts | 4 +- .../src/lang/ast/field-space/static-space.ts | 9 +- .../lang/ast/query-items/field-declaration.ts | 70 ++------- .../malloy/src/lang/ast/typedesc-utils.ts | 27 ++-- .../src/lang/test/field-symbols.spec.ts | 2 +- packages/malloy/src/lang/test/imports.spec.ts | 6 +- packages/malloy/src/lang/test/query.spec.ts | 4 +- .../malloy/src/lang/test/test-translator.ts | 11 +- packages/malloy/src/malloy.ts | 3 +- .../src/model/composite_source_utils.ts | 4 +- packages/malloy/src/model/malloy_query.ts | 5 +- packages/malloy/src/model/malloy_types.ts | 143 ++++++++++++++---- .../src/databases/all/compound-atomic.spec.ts | 10 -- 26 files changed, 262 insertions(+), 291 deletions(-) diff --git a/packages/malloy-db-bigquery/src/bigquery_connection.ts b/packages/malloy-db-bigquery/src/bigquery_connection.ts index 5586f82b3..7089c260b 100644 --- a/packages/malloy-db-bigquery/src/bigquery_connection.ts +++ b/packages/malloy-db-bigquery/src/bigquery_connection.ts @@ -35,7 +35,7 @@ import {ResourceStream} from '@google-cloud/paginator'; import * as googleCommon from '@google-cloud/common'; import {GaxiosError} from 'gaxios'; import { - arrayEachFields, + mkArrayDef, Connection, ConnectionConfig, Malloy, @@ -520,14 +520,7 @@ export class BigQueryConnection // Malloy treats repeated values as an array of scalars. const malloyType = this.dialect.sqlTypeToMalloyType(type); if (malloyType) { - const arrayField: StructDef = { - ...structShared, - type: 'array', - elementTypeDef: malloyType, - join: 'many', - fields: arrayEachFields(malloyType), - }; - structDef.fields.push(arrayField); + structDef.fields.push(mkArrayDef(malloyType, name, this.dialectName)); } } else if (isRecord) { const ifRepeatedRecord: StructDef = { diff --git a/packages/malloy-db-duckdb/src/duckdb.spec.ts b/packages/malloy-db-duckdb/src/duckdb.spec.ts index f63956fe4..77c3efc94 100644 --- a/packages/malloy-db-duckdb/src/duckdb.spec.ts +++ b/packages/malloy-db-duckdb/src/duckdb.spec.ts @@ -23,7 +23,7 @@ import {DuckDBCommon} from './duckdb_common'; import {DuckDBConnection} from './duckdb_connection'; -import {arrayEachFields, SQLSourceDef, StructDef} from '@malloydata/malloy'; +import {SQLSourceDef, StructDef, mkArrayDef} from '@malloydata/malloy'; import {describeIfDatabaseAvailable} from '@malloydata/malloy/test'; const [describe] = describeIfDatabaseAvailable(['duckdb']); @@ -132,14 +132,9 @@ describe('DuckDBConnection', () => { it('parses arrays', () => { const structDef = makeStructDef(); connection.fillStructDefFromTypeMap(structDef, {test: ARRAY_SCHEMA}); - expect(structDef.fields[0]).toEqual({ - name: 'test', - type: 'array', - elementTypeDef: intTyp, - join: 'many', - dialect: 'duckdb', - fields: arrayEachFields({type: 'number', numberType: 'integer'}), - }); + expect(structDef.fields[0]).toEqual( + mkArrayDef({type: 'number', numberType: 'integer'}, 'test', 'duckdb') + ); }); it('parses inline', () => { diff --git a/packages/malloy-db-duckdb/src/duckdb_common.ts b/packages/malloy-db-duckdb/src/duckdb_common.ts index e0e1e548a..92af07c74 100644 --- a/packages/malloy-db-duckdb/src/duckdb_common.ts +++ b/packages/malloy-db-duckdb/src/duckdb_common.ts @@ -35,7 +35,7 @@ import { DuckDBDialect, SQLSourceDef, TableSourceDef, - arrayEachFields, + mkArrayDef, } from '@malloydata/malloy'; import {BaseConnection} from '@malloydata/malloy/connection'; @@ -240,15 +240,7 @@ export abstract class DuckDBCommon } else { if (arrayMatch) { malloyType = this.dialect.sqlTypeToMalloyType(duckDBType); - const innerStructDef: StructDef = { - type: 'array', - elementTypeDef: malloyType, - name, - dialect: this.dialectName, - join: 'many', - fields: arrayEachFields(malloyType), - }; - structDef.fields.push(innerStructDef); + structDef.fields.push(mkArrayDef(malloyType, name, this.dialectName)); } else { structDef.fields.push({...malloyType, name}); } diff --git a/packages/malloy-db-postgres/src/postgres_connection.ts b/packages/malloy-db-postgres/src/postgres_connection.ts index 20fe59f6b..bf9bf4af1 100644 --- a/packages/malloy-db-postgres/src/postgres_connection.ts +++ b/packages/malloy-db-postgres/src/postgres_connection.ts @@ -42,9 +42,9 @@ import { RunSQLOptions, SQLSourceDef, TableSourceDef, - arrayEachFields, StreamingConnection, StructDef, + mkArrayDef, } from '@malloydata/malloy'; import {BaseConnection} from '@malloydata/malloy/connection'; @@ -237,14 +237,7 @@ export class PostgresConnection const elementType = this.dialect.sqlTypeToMalloyType( row['element_type'] as string ); - structDef.fields.push({ - type: 'array', - elementTypeDef: elementType, - name, - dialect: this.dialectName, - join: 'many', - fields: arrayEachFields(elementType), - }); + structDef.fields.push(mkArrayDef(elementType, name, this.dialectName)); } else { const malloyType = this.dialect.sqlTypeToMalloyType(postgresDataType); structDef.fields.push({...malloyType, name}); diff --git a/packages/malloy-db-snowflake/src/snowflake_connection.ts b/packages/malloy-db-snowflake/src/snowflake_connection.ts index a412ace8c..bd7b5d308 100644 --- a/packages/malloy-db-snowflake/src/snowflake_connection.ts +++ b/packages/malloy-db-snowflake/src/snowflake_connection.ts @@ -36,12 +36,12 @@ import { QueryDataRow, SnowflakeDialect, TestableConnection, - arrayEachFields, TinyParser, Dialect, - FieldDef, RecordDef, - ArrayTypeDef, + mkArrayDef, + AtomicFieldDef, + ArrayDef, } from '@malloydata/malloy'; import {BaseConnection} from '@malloydata/malloy/connection'; @@ -75,7 +75,7 @@ class SnowField { readonly type: string, readonly dialect: Dialect ) {} - fieldDef(): FieldDef { + fieldDef(): AtomicFieldDef { return { ...this.dialect.sqlTypeToMalloyType(this.type), name: this.name, @@ -102,15 +102,15 @@ class SnowObject extends SnowField { super(name, 'object', d); } - get fields(): FieldDef[] { - const fields: FieldDef[] = []; + get fields(): AtomicFieldDef[] { + const fields: AtomicFieldDef[] = []; for (const [_, fieldObj] of this.fieldMap) { fields.push(fieldObj.fieldDef()); } return fields; } - fieldDef() { + fieldDef(): RecordDef { const rec: RecordDef = { type: 'record', name: this.name, @@ -171,26 +171,27 @@ class SnowArray extends SnowField { } } - fieldDef(): ArrayTypeDef { - const arr: ArrayTypeDef = { - type: 'array', - name: this.name, - join: 'many', - dialect: this.dialect.name, - elementTypeDef: {type: 'string'}, - fields: [], - }; + fieldDef(): ArrayDef { if (this.objectChild) { - arr.fields = this.objectChild.fieldDef().fields; - arr.elementTypeDef = {type: 'record_element'}; - } else if (this.arrayChild) { - arr.elementTypeDef = this.arrayChild.fieldDef(); - arr.fields = arrayEachFields(arr.elementTypeDef); - } else { - arr.elementTypeDef = this.dialect.sqlTypeToMalloyType(this.arrayOf); - arr.fields = arrayEachFields(arr.elementTypeDef); + const t = mkArrayDef( + {type: 'record', fields: this.objectChild.fields}, + this.name, + this.dialect.name + ); + return t; + } + if (this.arrayChild) { + return mkArrayDef( + this.arrayChild.fieldDef(), + this.name, + this.dialect.name + ); } - return arr; + return mkArrayDef( + this.dialect.sqlTypeToMalloyType(this.arrayOf), + this.name, + this.dialect.name + ); } walk(path: PathChain, fieldType: string) { diff --git a/packages/malloy-db-trino/src/trino_connection.spec.ts b/packages/malloy-db-trino/src/trino_connection.spec.ts index 38410255f..c3c6a8e77 100644 --- a/packages/malloy-db-trino/src/trino_connection.spec.ts +++ b/packages/malloy-db-trino/src/trino_connection.spec.ts @@ -21,7 +21,7 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -import {arrayEachFields, AtomicTypeDef, FieldDef} from '@malloydata/malloy'; +import {AtomicTypeDef, FieldDef} from '@malloydata/malloy'; import {TrinoConnection, TrinoExecutor} from '.'; // array(varchar) is array @@ -64,22 +64,15 @@ describe('Trino connection', () => { describe('schema parser', () => { it('parses arrays', () => { expect(connection.malloyTypeFromTrinoType('test', ARRAY_SCHEMA)).toEqual({ - 'name': 'test', - 'type': 'array', - 'dialect': 'trino', - 'elementTypeDef': intType, - 'join': 'many', - 'fields': arrayEachFields(intType), + type: 'array', + elementTypeDef: intType, }); }); it('parses inline', () => { expect(connection.malloyTypeFromTrinoType('test', INLINE_SCHEMA)).toEqual( { - 'name': 'test', 'type': 'record', - 'dialect': 'trino', - 'join': 'one', 'fields': recordSchema, } ); @@ -88,11 +81,8 @@ describe('Trino connection', () => { it('parses nested', () => { expect(connection.malloyTypeFromTrinoType('test', NESTED_SCHEMA)).toEqual( { - 'name': 'test', 'type': 'array', 'elementTypeDef': {type: 'record_element'}, - 'dialect': 'trino', - 'join': 'many', 'fields': recordSchema, } ); @@ -106,11 +96,8 @@ describe('Trino connection', () => { it('parses deep nesting', () => { expect(connection.malloyTypeFromTrinoType('test', DEEP_SCHEMA)).toEqual({ - 'name': 'test', 'type': 'array', - 'dialect': 'trino', 'elementTypeDef': {type: 'record_element'}, - 'join': 'many', 'fields': [ {'name': 'a', ...doubleType}, { diff --git a/packages/malloy-db-trino/src/trino_connection.ts b/packages/malloy-db-trino/src/trino_connection.ts index 77ec06438..f58b0f58b 100644 --- a/packages/malloy-db-trino/src/trino_connection.ts +++ b/packages/malloy-db-trino/src/trino_connection.ts @@ -37,15 +37,16 @@ import { TableSourceDef, SQLSourceDef, AtomicTypeDef, - ArrayDef, + mkFieldDef, + isScalarArrayType, RepeatedRecordTypeDef, RecordTypeDef, - arrayEachFields, isRepeatedRecord, Dialect, ArrayTypeDef, FieldDef, TinyParser, + isRepeatedRecordType, } from '@malloydata/malloy'; import {BaseConnection} from '@malloydata/malloy/connection'; @@ -229,18 +230,19 @@ export abstract class TrinoPrestoConnection return data as unknown[]; } - convertRow(structDef: StructDef, rawRow: unknown) { + convertRow(fields: FieldDef[], rawRow: unknown) { const retRow = {}; const row = this.unpackArray(rawRow); - for (let i = 0; i < structDef.fields.length; i++) { - const field = structDef.fields[i]; + for (let i = 0; i < fields.length; i++) { + const field = fields[i]; if (field.type === 'record') { - retRow[field.name] = this.convertRow(field, row[i]); + retRow[field.name] = this.convertRow(field.fields, row[i]); } else if (isRepeatedRecord(field)) { - retRow[field.name] = this.convertNest(field, row[i]); + retRow[field.name] = this.convertNest(field.fields, row[i]); } else if (field.type === 'array') { - retRow[field.name] = this.convertNest(field, row[i]); + // mtoy todo don't understand this line actually + retRow[field.name] = this.convertNest(field.fields.slice(0, 1), row[i]); } else { retRow[field.name] = row[i] ?? null; } @@ -249,12 +251,12 @@ export abstract class TrinoPrestoConnection return retRow; } - convertNest(structDef: StructDef, _data: unknown) { + convertNest(fields: FieldDef[], _data: unknown) { const data = this.unpackArray(_data); const ret: unknown[] = []; const rows = (data === null || data === undefined ? [] : data) as unknown[]; for (const row of rows) { - ret.push(this.convertRow(structDef, row)); + ret.push(this.convertRow(fields, row)); } return ret; } @@ -295,12 +297,11 @@ export abstract class TrinoPrestoConnection private resultRow(colSchema: AtomicTypeDef, rawRow: unknown) { if (colSchema.type === 'record') { - return this.convertRow(colSchema, rawRow); - } else if (colSchema.type === 'array') { + return this.convertRow(colSchema.fields, rawRow); + } else if (isRepeatedRecordType(colSchema)) { + return this.convertNest(colSchema.fields, rawRow) as QueryValue; + } else if (isScalarArrayType(colSchema)) { const elType = colSchema.elementTypeDef; - if (elType.type === 'record_element') { - return this.convertNest(colSchema, rawRow) as QueryValue; - } let theArray = this.unpackArray(rawRow); if (elType.type === 'array') { theArray = theArray.map(el => this.resultRow(elType, el)); @@ -408,21 +409,14 @@ export abstract class TrinoPrestoConnection if (innerType.type === 'record') { const complexStruct: RepeatedRecordTypeDef = { type: 'array', - name, elementTypeDef: {type: 'record_element'}, - dialect: this.dialectName, - join: 'many', fields: innerType.fields, }; return complexStruct; } else { - const arrayStruct: ArrayDef = { + const arrayStruct: ArrayTypeDef = { type: 'array', - name, elementTypeDef: innerType, - dialect: this.dialectName, - join: 'many', - fields: arrayEachFields(innerType), }; return arrayStruct; } @@ -433,9 +427,6 @@ export abstract class TrinoPrestoConnection const innerTypes = this.splitColumns(structMatch[3]); const recordType: RecordTypeDef = { type: 'record', - name, - dialect: this.dialectName, - join: 'one', fields: [], }; for (let innerType of innerTypes) { @@ -454,7 +445,9 @@ export abstract class TrinoPrestoConnection innerName, innerTrinoType ); - recordType.fields.push({...innerMalloyType, name: innerName}); + recordType.fields.push( + mkFieldDef(innerMalloyType, innerName, this.dialectName) + ); } } return recordType; @@ -468,7 +461,7 @@ export abstract class TrinoPrestoConnection const type = row[4] || row[1]; const malloyType = this.malloyTypeFromTrinoType(name, type); // console.log('>', row, '\n<', malloyType); - structDef.fields.push({name, ...malloyType}); + structDef.fields.push(mkFieldDef(malloyType, name, this.dialectName)); } } @@ -675,7 +668,7 @@ class PrestoExplainParser extends TinyParser { const name = fieldNames[nameIndex]; this.next('id', ':'); const nextType = this.typeDef(); - fields.push({...nextType, name}); + fields.push(mkFieldDef(nextType, name, this.dialect.name)); const sep = this.next(); if (sep.text === ',') { continue; @@ -704,7 +697,7 @@ class PrestoExplainParser extends TinyParser { for (;;) { const name = this.next('quoted_name'); const getDef = this.typeDef(); - fields.push({...getDef, name: name.text}); + fields.push(mkFieldDef(getDef, name.text, this.dialect.name)); const sep = this.next(); if (sep.text === ')') { break; @@ -715,26 +708,19 @@ class PrestoExplainParser extends TinyParser { } const def: RecordTypeDef = { type: 'record', - name: '', - join: 'one', - dialect: this.dialect.name, fields, }; return def; } else if (typToken.text === 'array' && this.next('(')) { const elType = this.typeDef(); this.next(')'); - const def: ArrayTypeDef = { - type: 'array', - name: '', - dialect: this.dialect.name, - join: 'many', - elementTypeDef: - elType.type === 'record' ? {type: 'record_element'} : elType, - fields: - elType.type === 'record' ? elType.fields : arrayEachFields(elType), - }; - return def; + return elType.type === 'record' + ? { + type: 'array', + elementTypeDef: {type: 'record_element'}, + fields: elType.fields, + } + : {type: 'array', elementTypeDef: elType}; } else if (typToken.type === 'id') { const sqlType = typToken.text; const def = this.dialect.sqlTypeToMalloyType(sqlType); diff --git a/packages/malloy/src/dialect/snowflake/snowflake.ts b/packages/malloy/src/dialect/snowflake/snowflake.ts index e710faea5..a1168935e 100644 --- a/packages/malloy/src/dialect/snowflake/snowflake.ts +++ b/packages/malloy/src/dialect/snowflake/snowflake.ts @@ -40,6 +40,8 @@ import { ArrayLiteralNode, RecordLiteralNode, isAtomic, + isRepeatedRecordType, + isScalarArrayType, } from '../../model/malloy_types'; import { DialectFunctionOverloadDef, @@ -488,8 +490,7 @@ ${indent(sql)} } } else if ( malloyType.type === 'record' || - (malloyType.type === 'array' && - malloyType.elementTypeDef.type === 'record_element') + isRepeatedRecordType(malloyType) ) { const sqlFields = malloyType.fields.reduce((ret, f) => { if (isAtomic(f)) { @@ -505,10 +506,7 @@ ${indent(sql)} return malloyType.type === 'record' ? recordScehma : `ARRAY(${recordScehma})`; - } else if ( - malloyType.type === 'array' && - malloyType.elementTypeDef.type !== 'record_element' - ) { + } else if (isScalarArrayType(malloyType)) { return `ARRAY(${this.malloyTypeToSQLType(malloyType.elementTypeDef)})`; } return malloyType.type; diff --git a/packages/malloy/src/doc/fielddef.md b/packages/malloy/src/doc/fielddef.md index 1b27f0391..adde0595e 100644 --- a/packages/malloy/src/doc/fielddef.md +++ b/packages/malloy/src/doc/fielddef.md @@ -72,6 +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. ## Views diff --git a/packages/malloy/src/index.ts b/packages/malloy/src/index.ts index ecd020786..773a543a7 100644 --- a/packages/malloy/src/index.ts +++ b/packages/malloy/src/index.ts @@ -117,17 +117,21 @@ export type { ArrayLiteralNode, } from './model'; export { - arrayEachFields, isRepeatedRecord, isSourceDef, // Used in Composer Demo Segment, isLeafAtomic, - isJoined, + isJoinedField, isJoinedSource, isSamplingEnable, isSamplingPercent, isSamplingRows, + isRepeatedRecordType, + isScalarArrayType, + isScalarArray, + mkArrayDef, + mkFieldDef, expressionIsAggregate, expressionIsAnalytic, expressionIsCalculation, diff --git a/packages/malloy/src/lang/ast/expressions/expr-aggregate-function.ts b/packages/malloy/src/lang/ast/expressions/expr-aggregate-function.ts index dbd104d47..49b475b8d 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-aggregate-function.ts +++ b/packages/malloy/src/lang/ast/expressions/expr-aggregate-function.ts @@ -29,7 +29,7 @@ import { AggregateExpr, Expr, hasExpression, - isJoined, + isJoinedField, isAtomic, } from '../../../model/malloy_types'; import {exprWalk} from '../../../model/utils'; @@ -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) && !isJoined(field)) { + if (isAtomic(field) && !isJoinedField(field)) { if (hasExpression(field)) { const defUsage = getJoinUsage(def.fs, field.e); result.push(...defUsage.map(r => [...def.joinPath, ...r])); diff --git a/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts b/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts index cd2379762..a7e9836a2 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts +++ b/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts @@ -5,12 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import { - ArrayLiteralNode, - arrayEachFields, - ArrayTypeDef, - Expr, -} from '../../../model'; +import {ArrayLiteralNode, ArrayTypeDef, Expr} from '../../../model'; import {ExprValue, computedExprValue} from '../types/expr-value'; import {ExpressionDef} from '../types/expression-def'; import {FieldSpace} from '../types/field-space'; @@ -49,20 +44,17 @@ export class ArrayLiteral extends ExpressionDef { } } const elementTypeDef = TDU.atomicDef(firstValue || {type: 'number'}); - const typeDef: ArrayTypeDef = { - type: 'array', - join: 'many', - name: '', - dialect: fs.dialectName(), - elementTypeDef: - elementTypeDef.type !== 'record' - ? elementTypeDef - : {type: 'record_element'}, - fields: - elementTypeDef.type === 'record' - ? elementTypeDef.fields - : arrayEachFields(elementTypeDef), - }; + const typeDef: ArrayTypeDef = + elementTypeDef.type === 'record' + ? { + type: 'array', + elementTypeDef: {type: 'record_element'}, + fields: elementTypeDef.fields, + } + : { + type: 'array', + elementTypeDef, + }; const aLit: ArrayLiteralNode = { node: 'arrayLiteral', kids: {values}, diff --git a/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts b/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts index 2e7d0a525..4c33474c4 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts +++ b/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {TD, RecordLiteralNode} from '../../../model'; +import {TD, RecordLiteralNode, mkFieldDef} from '../../../model'; import {ExprValue, computedExprValue} from '../types/expr-value'; import {ExpressionDef} from '../types/expression-def'; import {FieldSpace} from '../types/field-space'; @@ -35,10 +35,7 @@ export class RecordLiteral extends ExpressionDef { node: 'recordLiteral', kids: {}, typeDef: { - name: '', type: 'record', - join: 'one', - dialect: fs.dialectName(), fields: [], }, }; @@ -48,7 +45,9 @@ export class RecordLiteral extends ExpressionDef { if (TD.isAtomic(xVal)) { dependents.push(xVal); recLit.kids[el.key] = xVal.value; - recLit.typeDef.fields.push({...TDU.atomicDef(xVal), name: el.key}); + recLit.typeDef.fields.push( + mkFieldDef(TDU.atomicDef(xVal), el.key, fs.dialectName()) + ); } else { this.logError( 'illegal-record-property-type', diff --git a/packages/malloy/src/lang/ast/field-space/reference-field.ts b/packages/malloy/src/lang/ast/field-space/reference-field.ts index 2e6220518..86f7e7555 100644 --- a/packages/malloy/src/lang/ast/field-space/reference-field.ts +++ b/packages/malloy/src/lang/ast/field-space/reference-field.ts @@ -26,6 +26,7 @@ import { QueryFieldDef, TD, TypeDesc, + mkFieldDef, } from '../../../model/malloy_types'; import * as TDU from '../typedesc-utils'; import {FieldReference} from '../query-items/field-references'; @@ -69,8 +70,7 @@ export class ReferenceField extends SpaceField { const foundType = check.found.typeDesc(); if (TD.isAtomic(foundType)) { this.queryFieldDef = { - ...TDU.atomicDef(foundType), - name: path[0], + ...mkFieldDef(TDU.atomicDef(foundType), path[0], fs.dialectName()), e: {node: 'parameter', path}, }; } else { diff --git a/packages/malloy/src/lang/ast/field-space/static-space.ts b/packages/malloy/src/lang/ast/field-space/static-space.ts index b1cf75295..8c9cdc44a 100644 --- a/packages/malloy/src/lang/ast/field-space/static-space.ts +++ b/packages/malloy/src/lang/ast/field-space/static-space.ts @@ -27,7 +27,7 @@ import { FieldDef, StructDef, SourceDef, - isJoined, + isJoinedField, isTurtleDef, isSourceDef, JoinFieldDef, @@ -71,7 +71,7 @@ export class StaticSpace implements FieldSpace { } defToSpaceField(from: FieldDef): SpaceField { - if (isJoined(from)) { + if (isJoinedField(from)) { return new StructSpaceField(from); } else if (isTurtleDef(from)) { return new IRViewField(this, from); @@ -151,7 +151,10 @@ export class StaticSpace implements FieldSpace { if (found instanceof SpaceField) { const definition = found.fieldDef(); if (definition) { - if (!(found instanceof StructSpaceFieldBase) && isJoined(definition)) { + if ( + !(found instanceof StructSpaceFieldBase) && + isJoinedField(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. diff --git a/packages/malloy/src/lang/ast/query-items/field-declaration.ts b/packages/malloy/src/lang/ast/query-items/field-declaration.ts index b13a85c9d..e720812d8 100644 --- a/packages/malloy/src/lang/ast/query-items/field-declaration.ts +++ b/packages/malloy/src/lang/ast/query-items/field-declaration.ts @@ -29,9 +29,9 @@ import { TypeDesc, FieldDef, AtomicFieldDef, - TemporalTypeDef, isAtomic, FieldDefType, + mkFieldDef, } from '../../../model/malloy_types'; import * as TDU from '../typedesc-utils'; @@ -135,62 +135,20 @@ export abstract class AtomicFieldDeclaration } if (isAtomicFieldType(exprValue.type) && exprValue.type !== 'error') { this.typecheckExprValue(exprValue); - let ret: AtomicFieldDef; - switch (exprValue.type) { - case 'date': - case 'timestamp': { - const timeRet: TemporalTypeDef & AtomicFieldDef = { - name: exprName, - type: exprValue.type, - location: this.location, - e: exprValue.value, - compositeFieldUsage: exprValue.compositeFieldUsage, - }; - if (isGranularResult(exprValue)) { - timeRet.timeframe = exprValue.timeframe; - } - ret = timeRet; - break; - } - case 'json': - case 'boolean': - case 'string': - case 'number': - case 'sql native': { - ret = { - type: exprValue.type, - name: exprName, - location: this.location, - e: exprValue.value, - compositeFieldUsage: exprValue.compositeFieldUsage, - }; - break; - } - case 'record': { - ret = { - type: 'record', - name: exprName, - location: this.location, - join: 'one', - fields: exprValue.fields, - e: exprValue.value, - compositeFieldUsage: exprValue.compositeFieldUsage, - dialect: exprFS.dialectName(), - }; - break; - } - case 'array': { - ret = { - type: 'array', - elementTypeDef: exprValue.elementTypeDef, - name: exprName, - join: 'many', - fields: exprValue.fields, - e: exprValue.value, - dialect: exprValue.dialect, - }; - } + const ret = mkFieldDef( + TDU.atomicDef(exprValue), + exprName, + exprFS.dialectName() + ); + if ( + (ret.type === 'date' || ret.type === 'timestamp') && + isGranularResult(exprValue) + ) { + ret.timeframe = exprValue.timeframe; } + ret.location = this.location; + ret.e = exprValue.value; + ret.compositeFieldUsage = exprValue.compositeFieldUsage; if (exprValue.expressionType) { ret.expressionType = exprValue.expressionType; } diff --git a/packages/malloy/src/lang/ast/typedesc-utils.ts b/packages/malloy/src/lang/ast/typedesc-utils.ts index 1c130ee44..67e28c30a 100644 --- a/packages/malloy/src/lang/ast/typedesc-utils.ts +++ b/packages/malloy/src/lang/ast/typedesc-utils.ts @@ -27,6 +27,7 @@ import { expressionIsScalar, ExpressionType, ExpressionValueType, + isRepeatedRecordType, TD, TypeDesc, } from '../../model'; @@ -144,23 +145,19 @@ export function atomicDef(td: AtomicTypeDef | TypeDesc): AtomicTypeDef { if (TD.isAtomic(td)) { switch (td.type) { case 'array': { - return { - name: '', - type: 'array', - join: 'many', - elementTypeDef: td.elementTypeDef, - dialect: td.dialect, - fields: td.fields, - }; + return isRepeatedRecordType(td) + ? { + type: 'array', + elementTypeDef: td.elementTypeDef, + fields: td.fields, + } + : { + type: 'array', + elementTypeDef: td.elementTypeDef, + }; } case 'record': { - return { - name: '', - type: 'record', - join: 'one', - dialect: td.dialect, - fields: td.fields, - }; + return {type: 'record', fields: td.fields}; } case 'number': { return td.numberType diff --git a/packages/malloy/src/lang/test/field-symbols.spec.ts b/packages/malloy/src/lang/test/field-symbols.spec.ts index 1e74d6cd8..99d6edee4 100644 --- a/packages/malloy/src/lang/test/field-symbols.spec.ts +++ b/packages/malloy/src/lang/test/field-symbols.spec.ts @@ -112,7 +112,7 @@ describe('structdef comprehension', () => { }); test('import repeated record', () => { - const field: model.ArrayDef = { + const field: model.LeafArrayDef = { name: 't', type: 'array', dialect: 'standardsql', diff --git a/packages/malloy/src/lang/test/imports.spec.ts b/packages/malloy/src/lang/test/imports.spec.ts index 920b9e479..ced1e1368 100644 --- a/packages/malloy/src/lang/test/imports.spec.ts +++ b/packages/malloy/src/lang/test/imports.spec.ts @@ -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 {isJoined} from '../../model'; +import {isJoinedField} from '../../model'; import './parse-expects'; import {TestTranslator, errorMessage, model} from './test-translator'; import escapeRegEx from 'lodash/escapeRegExp'; @@ -91,8 +91,8 @@ source: botProjQSrc is botProjQ const maybeField = newSrc?.fields.find(f => f.name === 'b'); expect(maybeField).toBeDefined(); const f = maybeField!; - expect(isJoined(f)).toBeTruthy(); - if (isJoined(f)) { + expect(isJoinedField(f)).toBeTruthy(); + if (isJoinedField(f)) { expect(f.type).toBe('query_source'); if (f.type === 'query_source') { expect(typeof f.query.structRef).not.toBe('string'); diff --git a/packages/malloy/src/lang/test/query.spec.ts b/packages/malloy/src/lang/test/query.spec.ts index bce500906..a28c9fe16 100644 --- a/packages/malloy/src/lang/test/query.spec.ts +++ b/packages/malloy/src/lang/test/query.spec.ts @@ -36,7 +36,7 @@ import { QueryFieldDef, QuerySegment, expressionIsCalculation, - isJoined, + isJoinedField, isQuerySegment, isAtomic, } from '../../model'; @@ -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(isJoined(q.extendSource[0])).toBeTruthy(); + expect(isJoinedField(q.extendSource[0])).toBeTruthy(); expect(q.extendSource[0].type).toBe('table'); } else { fail('Did not generate extendSource'); diff --git a/packages/malloy/src/lang/test/test-translator.ts b/packages/malloy/src/lang/test/test-translator.ts index cc1d9e406..1acbf6e45 100644 --- a/packages/malloy/src/lang/test/test-translator.ts +++ b/packages/malloy/src/lang/test/test-translator.ts @@ -41,8 +41,8 @@ import { TableSourceDef, SQLSourceDef, SQLSentence, - arrayEachFields, NumberTypeDef, + mkArrayDef, } from '../../model/malloy_types'; import {ExpressionDef, MalloyElement} from '../ast'; import {NameSpace} from '../ast/types/name-space'; @@ -98,14 +98,7 @@ const mockSchema: Record = { matrixOperation: 'left', dialect: 'standardsql', }, - { - type: 'array', - name: 'ais', - elementTypeDef: intType, - join: 'many', - fields: arrayEachFields(intType), - dialect: 'standardsql', - }, + mkArrayDef(intType, 'ais', 'standardsql'), ], }, 'malloytest.carriers': { diff --git a/packages/malloy/src/malloy.ts b/packages/malloy/src/malloy.ts index a22ef1bfb..513c4c456 100644 --- a/packages/malloy/src/malloy.ts +++ b/packages/malloy/src/malloy.ts @@ -68,6 +68,7 @@ import { SourceDef, isSourceDef, QueryToMaterialize, + isJoinedField, isJoined, } from './model'; import { @@ -1572,7 +1573,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 (isJoined(fieldDef)) { + if (isJoinedField(fieldDef)) { return [name, new ExploreField(fieldDef, this, sourceField)]; } else if (fieldDef.type === 'turtle') { return [name, new QueryField(fieldDef, this, sourceField)]; diff --git a/packages/malloy/src/model/composite_source_utils.ts b/packages/malloy/src/model/composite_source_utils.ts index 00af6de5c..a87fe0cff 100644 --- a/packages/malloy/src/model/composite_source_utils.ts +++ b/packages/malloy/src/model/composite_source_utils.ts @@ -10,7 +10,7 @@ import { CompositeFieldUsage, FieldDef, isJoinable, - isJoined, + isJoinedField, isSourceDef, SourceDef, } from './malloy_types'; @@ -147,7 +147,7 @@ function _resolveCompositeSources( error: {code: 'composite_source_not_defined', data: {path: newPath}}, }; } - if (!isJoined(join) || !isSourceDef(join)) { + if (!isJoinedField(join) || !isSourceDef(join)) { return { error: {code: 'composite_source_not_a_join', data: {path: newPath}}, }; diff --git a/packages/malloy/src/model/malloy_query.ts b/packages/malloy/src/model/malloy_query.ts index ad7732851..1253b3365 100644 --- a/packages/malloy/src/model/malloy_query.ts +++ b/packages/malloy/src/model/malloy_query.ts @@ -98,7 +98,7 @@ import { isBaseTable, NestSourceDef, TimestampFieldDef, - isJoined, + isJoinedField, isJoinedSource, QueryResultDef, isScalarArray, @@ -113,6 +113,7 @@ import { JoinFieldDef, LeafAtomicDef, Expression, + isJoined, } from './malloy_types'; import {Connection} from '../connection/types'; @@ -1880,7 +1881,7 @@ class FieldInstanceResult implements FieldInstance { if (fi.fieldUsage.type === 'result') { if ( fi.f.fieldDef.type === 'turtle' || - isJoined(fi.f.fieldDef) || + isJoinedField(fi.f.fieldDef) || expressionIsAnalytic(fi.f.fieldDef.expressionType) ) { continue; diff --git a/packages/malloy/src/model/malloy_types.ts b/packages/malloy/src/model/malloy_types.ts index 4cf454332..03bcaf9b5 100644 --- a/packages/malloy/src/model/malloy_types.ts +++ b/packages/malloy/src/model/malloy_types.ts @@ -689,21 +689,76 @@ export interface NativeUnsupportedTypeDef { export type NativeUnsupportedFieldDef = NativeUnsupportedTypeDef & AtomicFieldDef; -export interface ArrayTypeDef extends JoinBase, StructDefBase { +export interface LeafArrayTypeDef { + type: 'array'; + elementTypeDef: Exclude; +} +export interface LeafArrayDef + extends LeafArrayTypeDef, + StructDefBase, + JoinBase, + FieldBase { type: 'array'; - elementTypeDef: Exclude | RecordElementTypeDef; join: 'many'; } -export type ArrayDef = ArrayTypeDef & AtomicFieldDef; -export function arrayEachFields(arrayOf: AtomicTypeDef): AtomicFieldDef[] { - return [ - {...arrayOf, e: {node: 'field', path: ['value']}, name: 'each'}, - {...arrayOf, name: 'value'}, - ]; +export function mkFieldDef( + atd: AtomicTypeDef, + name: string, + dialect: string +): AtomicFieldDef { + if (isScalarArrayType(atd)) { + return mkArrayDef(atd.elementTypeDef, name, dialect); + } + if (isRepeatedRecordType(atd)) { + const {type, fields, elementTypeDef} = atd; + return {type, fields, elementTypeDef, join: 'many', name, dialect}; + } + if (atd.type === 'record') { + const {type, fields} = atd; + return {type, fields, join: 'one', dialect, name}; + } + return {...atd, name}; +} + +export function mkArrayDef( + ofType: AtomicTypeDef, + name: string, + dialect: string +): ArrayDef { + if (ofType.type === 'record') { + return { + type: 'array', + join: 'many', + name, + dialect, + elementTypeDef: {type: 'record_element'}, + fields: ofType.fields, + }; + } + const valueEnt = mkFieldDef(ofType, 'value', dialect); + return { + type: 'array', + join: 'many', + name, + dialect, + elementTypeDef: ofType, + fields: [ + valueEnt, + {...valueEnt, name: 'each', e: {node: 'field', path: ['value']}}, + ], + }; } -export interface RecordTypeDef extends StructDefBase, JoinBase { +export interface RecordTypeDef { + type: 'record'; + fields: FieldDef[]; +} +export interface RecordDef + extends RecordTypeDef, + StructDefBase, + JoinBase, + FieldBase { type: 'record'; join: 'one'; } @@ -725,19 +780,44 @@ export interface RecordElementTypeDef { type: 'record_element'; } -export interface RepeatedRecordTypeDef extends ArrayDef { +export interface RepeatedRecordTypeDef { type: 'array'; elementTypeDef: RecordElementTypeDef; + fields: FieldDef[]; +} +export interface RepeatedRecordDef + extends RepeatedRecordTypeDef, + StructDefBase, + JoinBase, + FieldBase { + type: 'array'; join: 'many'; } +export type ArrayTypeDef = LeafArrayTypeDef | RepeatedRecordTypeDef; +export type ArrayDef = LeafArrayDef | RepeatedRecordDef; -export type RecordDef = RecordTypeDef & AtomicFieldDef; -export type RepeatedRecordDef = RepeatedRecordTypeDef & AtomicFieldDef; +export function isRepeatedRecordType( + td: AtomicTypeDef +): td is RepeatedRecordTypeDef { + return td.type === 'array' && td.elementTypeDef.type === 'record_element'; +} -export function isRepeatedRecord(fd: FieldDef): fd is RepeatedRecordDef { +export function isRepeatedRecord( + fd: FieldDef | QueryFieldDef | StructDef +): fd is RepeatedRecordDef { return fd.type === 'array' && fd.elementTypeDef.type === 'record_element'; } +export function isScalarArrayType(td: AtomicTypeDef): td is LeafArrayTypeDef { + return td.type === 'array' && td.elementTypeDef.type !== 'record_element'; +} + +export function isScalarArray( + fd: FieldDef | QueryFieldDef | StructDef +): fd is LeafArrayDef { + return fd.type === 'array' && fd.elementTypeDef.type !== 'record_element'; +} + export interface ErrorTypeDef { type: 'error'; } @@ -777,29 +857,32 @@ export type Joinable = | TableSourceDef | SQLSourceDef | QuerySourceDef + | RepeatedRecordDef | RecordDef | ArrayDef; -export type JoinFieldDef = JoinBase & Joinable; +export type JoinFieldDef = Joinable & JoinBase; export function isJoinable(sd: StructDef): sd is Joinable { return [ + 'composite', 'table', 'sql_select', 'query_source', 'array', 'record', - 'composite', ].includes(sd.type); } -export function isJoined( - fd: T -): fd is T & Joinable & JoinBase { +export function isJoinedField(fd: FieldDef): fd is JoinFieldDef { return 'join' in fd; } +export function isJoined(sd: StructDef): sd is JoinFieldDef { + return 'join' in sd; +} + export function isJoinedSource(sd: StructDef): sd is SourceDef & JoinBase { - return isSourceDef(sd) && isJoined(sd); + return isSourceDef(sd) && 'join' in sd; } export type DateUnit = 'day' | 'week' | 'month' | 'quarter' | 'year'; @@ -1145,7 +1228,7 @@ export type SourceDef = /** Is this the "FROM" table of a query tree */ export function isBaseTable(def: StructDef): def is SourceDef { - if (isJoined(def)) { + if (isJoinedSource(def)) { return false; } if (isSourceDef(def)) { @@ -1154,10 +1237,6 @@ export function isBaseTable(def: StructDef): def is SourceDef { return false; } -export function isScalarArray(def: FieldDef | StructDef) { - return def.type === 'array' && def.elementTypeDef.type !== 'record_element'; -} - export type StructDef = SourceDef | RecordDef | ArrayDef; // "NonAtomic" are types that a name lookup or a computation might @@ -1258,10 +1337,18 @@ export type LeafAtomicTypeDef = | JSONTypeDef | NativeUnsupportedTypeDef | ErrorTypeDef; -export type AtomicTypeDef = LeafAtomicTypeDef | ArrayTypeDef | RecordTypeDef; - export type LeafAtomicDef = LeafAtomicTypeDef & FieldBase; -export type AtomicFieldDef = AtomicTypeDef & FieldBase; + +export type AtomicTypeDef = + | LeafAtomicTypeDef + | LeafArrayTypeDef + | RecordTypeDef + | RepeatedRecordTypeDef; +export type AtomicFieldDef = + | LeafAtomicDef + | LeafArrayDef + | RecordDef + | RepeatedRecordDef; export function isLeafAtomic( fd: FieldDef | QueryFieldDef | AtomicTypeDef @@ -1278,7 +1365,7 @@ export function isLeafAtomic( } // Sources have fields like this ... -export type FieldDef = AtomicFieldDef | JoinFieldDef | TurtleDef; +export type FieldDef = LeafAtomicDef | JoinFieldDef | TurtleDef; export type FieldDefType = AtomicFieldType | 'turtle' | JoinElementType; // Queries have fields like this .. diff --git a/test/src/databases/all/compound-atomic.spec.ts b/test/src/databases/all/compound-atomic.spec.ts index 22c24b680..6fd6123e3 100644 --- a/test/src/databases/all/compound-atomic.spec.ts +++ b/test/src/databases/all/compound-atomic.spec.ts @@ -40,11 +40,7 @@ describe.each(runtimes.runtimeList)( node: 'arrayLiteral', typeDef: { type: 'array', - name: 'evens', - join: 'many', elementTypeDef: {type: 'number'}, - fields: [], - dialect: runtime.dialect.name, }, kids: {values: val.map(v => literalNum(v))}, }; @@ -63,9 +59,6 @@ describe.each(runtimes.runtimeList)( node: 'recordLiteral', typeDef: { type: 'record', - name: 'evens', - join: 'one', - dialect: runtime.dialect.name, fields, }, kids, @@ -372,14 +365,11 @@ describe.each(runtimes.runtimeList)( describe('repeated record', () => { const abType: ArrayTypeDef = { type: 'array', - dialect: runtime.dialect.name, - join: 'many', elementTypeDef: {type: 'record_element'}, fields: [ {name: 'a', type: 'number'}, {name: 'b', type: 'number'}, ], - name: '', }; const values = [ recordLiteral({a: 10, b: 11}),