Typedef cleanup cleanup (#2033)

* combine and smplify discriminators * LeafArray should be ScalarArray * cleanup naming for isTemporal * all hour our benevolent protectors
malloydata · Dec 5, 2024 · 893df28 · 893df28
1 parent 9cd1f69
commit 893df28
Show file tree

Hide file tree

Showing 20 changed files with 95 additions and 110 deletions.
diff --git a/packages/malloy-db-trino/src/trino_connection.ts b/packages/malloy-db-trino/src/trino_connection.ts
@@ -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';
@@ -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') {

diff --git a/packages/malloy/src/dialect/snowflake/snowflake.ts b/packages/malloy/src/dialect/snowflake/snowflake.ts
@@ -40,8 +40,8 @@ import {
   ArrayLiteralNode,
   RecordLiteralNode,
   isAtomic,
-  isRepeatedRecordType,
-  isScalarArrayType,
+  isRepeatedRecord,
+  isScalarArray,
 } from '../../model/malloy_types';
 import {
   DialectFunctionOverloadDef,
@@ -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;
@@ -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;

diff --git a/packages/malloy/src/doc/fielddef.md b/packages/malloy/src/doc/fielddef.md
@@ -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
 
@@ -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
diff --git a/packages/malloy/src/index.ts b/packages/malloy/src/index.ts
@@ -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,

diff --git a/packages/malloy/src/lang/ast/expressions/expr-aggregate-function.ts b/packages/malloy/src/lang/ast/expressions/expr-aggregate-function.ts
@@ -29,8 +29,8 @@ import {
   AggregateExpr,
   Expr,
   hasExpression,
-  isJoinedField,
   isAtomic,
+  isJoined,
 } 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) && !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]));

diff --git a/packages/malloy/src/lang/ast/expressions/expr-time-extract.ts b/packages/malloy/src/lang/ast/expressions/expr-time-extract.ts
@@ -24,7 +24,7 @@
 import {
   ExtractUnit,
   isExtractUnit,
-  isTemporalField,
+  isTemporalType,
   isTimestampUnit,
   mkTemporal,
   TD,
@@ -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}'`
@@ -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: {

diff --git a/packages/malloy/src/lang/ast/expressions/expr-time.ts b/packages/malloy/src/lang/ast/expressions/expr-time.ts
@@ -25,7 +25,7 @@ import {
   Expr,
   TemporalFieldType,
   TypecastExpr,
-  isTemporalField,
+  isTemporalType,
 } from '../../../model/malloy_types';
 
 import {FieldSpace} from '../types/field-space';
@@ -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;

diff --git a/packages/malloy/src/lang/ast/expressions/time-literal.ts b/packages/malloy/src/lang/ast/expressions/time-literal.ts
@@ -26,7 +26,7 @@ import {DateTime as LuxonDateTime} from 'luxon';
 import {
   TemporalFieldType,
   TimestampUnit,
-  isTemporalField,
+  isTemporalType,
   TimeLiteralNode,
 } from '../../../model/malloy_types';
 
@@ -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),

diff --git a/packages/malloy/src/lang/ast/field-space/static-space.ts b/packages/malloy/src/lang/ast/field-space/static-space.ts
@@ -27,8 +27,8 @@ import {
   FieldDef,
   StructDef,
   SourceDef,
-  isJoinedField,
-  isTurtleDef,
+  isJoined,
+  isTurtle,
   isSourceDef,
   JoinFieldDef,
 } from '../../../model/malloy_types';
@@ -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);
@@ -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.

diff --git a/packages/malloy/src/lang/ast/query-builders/reduce-builder.ts b/packages/malloy/src/lang/ast/query-builders/reduce-builder.ts
@@ -36,7 +36,7 @@ import {
   isPartialSegment,
   isQuerySegment,
   isReduceSegment,
-  isTemporalField,
+  isTemporalType,
 } from '../../../model/malloy_types';
 
 import {ErrorFactory} from '../error-factory';
@@ -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;

diff --git a/packages/malloy/src/lang/ast/typedesc-utils.ts b/packages/malloy/src/lang/ast/typedesc-utils.ts
@@ -27,7 +27,7 @@ import {
   expressionIsScalar,
   ExpressionType,
   ExpressionValueType,
-  isRepeatedRecordType,
+  isRepeatedRecord,
   TD,
   TypeDesc,
 } from '../../model';
@@ -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,

diff --git a/packages/malloy/src/lang/ast/types/expression-def.ts b/packages/malloy/src/lang/ast/types/expression-def.ts
@@ -25,7 +25,7 @@ import {
   Expr,
   TimestampUnit,
   isDateUnit,
-  isTemporalField,
+  isTemporalType,
   expressionIsAggregate,
   TD,
   LeafExpressionType,
@@ -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(
@@ -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) {
@@ -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;

diff --git a/packages/malloy/src/lang/ast/view-elements/reference-view.ts b/packages/malloy/src/lang/ast/view-elements/reference-view.ts
@@ -25,7 +25,7 @@ import {
   PipeSegment,
   SourceDef,
   isAtomic,
-  isTurtleDef,
+  isTurtle,
   sourceBase,
 } from '../../../model/malloy_types';
 import {ErrorFactory} from '../error-factory';
@@ -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(

diff --git 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.LeafArrayDef = {
+    const field: model.ScalarArrayDef = {
       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
@@ -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';
@@ -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');

diff --git 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,
-  isJoinedField,
+  isJoined,
   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(isJoinedField(q.extendSource[0])).toBeTruthy();
+          expect(isJoined(q.extendSource[0])).toBeTruthy();
           expect(q.extendSource[0].type).toBe('table');
         } else {
           fail('Did not generate extendSource');

diff --git a/packages/malloy/src/malloy.ts b/packages/malloy/src/malloy.ts
@@ -68,7 +68,6 @@ import {
   SourceDef,
   isSourceDef,
   QueryToMaterialize,
-  isJoinedField,
   isJoined,
 } from './model';
 import {
@@ -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)];