remove some todos

malloydata · Nov 27, 2024 · ee4bb9e · ee4bb9e
1 parent dc88851
commit ee4bb9e
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 18 deletions.
diff --git a/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts b/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts
@@ -5,7 +5,12 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {ArrayLiteralNode, arrayEachFields, ArrayTypeDef} from '../../../model';
+import {
+  ArrayLiteralNode,
+  arrayEachFields,
+  ArrayTypeDef,
+  Expr,
+} from '../../../model';
 import {ExprValue, computedExprValue} from '../types/expr-value';
 import {ExpressionDef} from '../types/expression-def';
 import {FieldSpace} from '../types/field-space';
@@ -19,17 +24,31 @@ export class ArrayLiteral extends ExpressionDef {
   }
 
   getExpression(fs: FieldSpace): ExprValue {
-    const values = this.elements.map(v => v.getExpression(fs));
-    const type = values[0];
-    const checkedValues = [values[0].value];
-    for (const newType of values.slice(1)) {
-      if (TDU.typeEq(type, newType)) {
-        checkedValues.push(newType.value);
-      } else {
-        throw new Error('mtoy todo array elements should be same type');
+    const values: Expr[] = [];
+    const fromValues: ExprValue[] = [];
+    let firstValue: ExprValue | undefined = undefined;
+    if (this.elements.length > 0) {
+      for (const nextElement of this.elements) {
+        const v = nextElement.getExpression(fs);
+        fromValues.push(v);
+        if (v.type === 'error') {
+          continue;
+        }
+        if (firstValue) {
+          if (!TDU.typeEq(firstValue, v)) {
+            nextElement.logError(
+              'array-values-incompatible',
+              'All array elements must be same type'
+            );
+            continue;
+          }
+        } else {
+          firstValue = v;
+        }
+        values.push(v.value);
       }
     }
-    const elementTypeDef = TDU.atomicDef(values[0]);
+    const elementTypeDef = TDU.atomicDef(firstValue || {type: 'number'});
     const typeDef: ArrayTypeDef = {
       type: 'array',
       join: 'many',
@@ -46,13 +65,13 @@ export class ArrayLiteral extends ExpressionDef {
     };
     const aLit: ArrayLiteralNode = {
       node: 'arrayLiteral',
-      kids: {values: checkedValues},
+      kids: {values},
       typeDef,
     };
     return computedExprValue({
       dataType: typeDef,
       value: aLit,
-      from: values,
+      from: fromValues,
     });
   }
 }
diff --git a/packages/malloy/src/lang/ast/field-space/reference-field.ts b/packages/malloy/src/lang/ast/field-space/reference-field.ts
@@ -74,7 +74,7 @@ export class ReferenceField extends SpaceField {
             e: {node: 'parameter', path},
           };
         } else {
-          // mtoy todo
+          // not sure what to do here, if we get here
           throw new Error('impossible turtle/join parameter');
         }
       } else {

diff --git a/packages/malloy/src/lang/ast/query-properties/filters.ts b/packages/malloy/src/lang/ast/query-properties/filters.ts
@@ -95,9 +95,6 @@ export class Filter
   ): FilterCondition | undefined {
     const fExpr = filter.filterCondition(fs);
 
-    // mtoy todo is having we never set then queryRefinementStage might be wrong
-    // ... calculations and aggregations must go last
-
     // Aggregates are ALSO checked at SQL generation time, but checking
     // here allows better reflection of errors back to user.
     if (this.havingClause !== undefined) {

diff --git a/packages/malloy/src/lang/grammar/MalloyParser.g4 b/packages/malloy/src/lang/grammar/MalloyParser.g4
@@ -574,7 +574,6 @@ fieldExpr
   | fieldPath DOT aggregate
       OPAREN fieldExpr? CPAREN                             # exprAggregate
   | OPAREN fieldExpr CPAREN                                # exprExpr
-  // mtoy todo should allow expr DOT expr, since expr can now be a record
   | fieldPath DOT id
       OPAREN ( argumentList? ) CPAREN                      # exprAggFunc
   | ((id (EXCLAM malloyType?)?) | timeframe)

diff --git a/packages/malloy/src/lang/parse-log.ts b/packages/malloy/src/lang/parse-log.ts
@@ -385,6 +385,7 @@ type MessageParameterTypes = {
   'or-choices-only': string;
   'sql-in': string;
   'dialect-cast-unsafe-only': string;
+  'array-values-incompatible': string;
 };
 
 export const MESSAGE_FORMATTERS: PartialErrorCodeMessageMap = {

diff --git a/test/src/databases/all/compound-atomic.spec.ts b/test/src/databases/all/compound-atomic.spec.ts
@@ -208,7 +208,6 @@ describe.each(runtimes.runtimeList)(
           {small: 0}
         );
       });
-      // mtoy todo remove this
       test('nested data looks like a record', async () => {
         await expect(`
           run: ${databaseName}.sql('SELECT 1 as ${quote('o')}') -> {