Only treat granular results as ranges with apply operator (#1686)

* tweak types for expr`test source`

* Only trreat granular results as ranges in apply

* move range apply to the apply operator

packages/malloy/src/lang/ast/expressions/apply.ts

@@ -24,6 +24,10 @@
 import {Comparison} from '../types/comparison';
 import {ExprCompare} from './expr-compare';
 import {ExpressionDef} from '../types/expression-def';
+import {ExprValue} from '../types/expr-value';
+import {FieldSpace} from '../types/field-space';
+import {isGranularResult} from '../types/granular-result';
+import {ExprGranularTime} from './expr-granular-time';
 
 export class Apply extends ExprCompare {
   elementType = 'apply';
@@ -33,4 +37,19 @@ export class Apply extends ExprCompare {
   ) {
     super(left, Comparison.EqualTo, right);
   }
+
+  getExpression(fs: FieldSpace): ExprValue {
+    let right = this.right;
+    if (!this.right.granular()) {
+      const rhs = this.right.requestExpression(fs);
+      if (rhs && isGranularResult(rhs)) {
+        // Need to wrap granular computations to get granular behavior
+        right = new ExprGranularTime(this.right, rhs.timeframe, false);
+      }
+    }
+    if (right instanceof ExprGranularTime) {
+      return right.toRange(fs).apply(fs, this.op, this.left);
+    }
+    return super.getExpression(fs);
+  }
 }

packages/malloy/src/lang/ast/expressions/expr-compare.ts

@@ -26,9 +26,7 @@ import {Comparison} from '../types/comparison';
 import {ExprValue} from '../types/expr-value';
 import {ExpressionDef} from '../types/expression-def';
 import {FieldSpace} from '../types/field-space';
-import {isGranularResult} from '../types/granular-result';
 import {BinaryBoolean} from './binary-boolean';
-import {ExprGranularTime} from './expr-granular-time';
 
 const compareTypes = {
   '~': [FT.stringT],
@@ -49,14 +47,6 @@ export class ExprCompare extends BinaryBoolean<Comparison> {
   }
 
   getExpression(fs: FieldSpace): ExprValue {
-    if (!this.right.granular()) {
-      const rhs = this.right.requestExpression(fs);
-      if (rhs && isGranularResult(rhs)) {
-        const newRight = new ExprGranularTime(this.right, rhs.timeframe, false);
-        return newRight.apply(fs, this.op, this.left);
-      }
-    }
-
     return this.right.apply(fs, this.op, this.left);
   }
 }

packages/malloy/src/lang/ast/expressions/expr-granular-time.ts

@@ -99,27 +99,27 @@ export class ExprGranularTime extends ExpressionDef {
     };
   }
 
-  apply(fs: FieldSpace, op: string, left: ExpressionDef): ExprValue {
-    return this.getRange(fs).apply(fs, op, left);
+  // apply(fs: FieldSpace, op: string, left: ExpressionDef): ExprValue {
+  //   return this.getRange(fs).apply(fs, op, left);
 
-    /*
-      write tests for each of these cases ....
+  //   /*
+  //     write tests for each of these cases ....
 
-      vt  rt  gt  use
-      dt  dt  dt  dateRange
-      dt  dt  ts  == or timeStampRange
-      dt  ts  dt  timestampRange
-      dt  ts  ts  timeStampRange
+  //     vt  rt  gt  use
+  //     dt  dt  dt  dateRange
+  //     dt  dt  ts  == or timeStampRange
+  //     dt  ts  dt  timestampRange
+  //     dt  ts  ts  timeStampRange
 
-      ts  ts  ts  timestampRange
-      ts  ts  dt  timestampRange
-      ts  dt  ts  timestampRange
-      ts  dt  dt  either
+  //     ts  ts  ts  timestampRange
+  //     ts  ts  dt  timestampRange
+  //     ts  dt  ts  timestampRange
+  //     ts  dt  dt  either
 
-    */
-  }
+  //   */
+  // }
 
-  protected getRange(fs: FieldSpace): Range {
+  toRange(fs: FieldSpace): Range {
     const begin = this.getExpression(fs);
     if (begin.dataType === 'timestamp') {
       const beginTS = ExprTime.fromValue('timestamp', begin);

packages/malloy/src/lang/test/expressions.spec.ts

@@ -21,7 +21,7 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import {isFieldTypeDef} from '../../model';
+import {Fragment, isFieldTypeDef} from '../../model';
 import {
   expr,
   TestTranslator,
@@ -32,6 +32,30 @@ import {
 } from './test-translator';
 import './parse-expects';
 
+/**
+ * Convert an expression to a string, any fragment which is not a string turns into a variable,
+ * useful for maybe checking simple code generation, but it sort of hints at an interesting matcher
+ * @param expr Compiled expression
+ * @returns A string with variables A,B,... substituded for non string elements
+ */
+function exprToString(expr: Fragment[]): string {
+  let varCode = 'A'.charCodeAt(0);
+  const vars: Record<string, string> = {};
+  return expr
+    .map(f => {
+      if (typeof f === 'string') {
+        return f;
+      }
+      const key = JSON.stringify(f);
+      if (!vars[key]) {
+        vars[key] = String.fromCharCode(varCode);
+        varCode += 1;
+      }
+      return vars[key];
+    })
+    .join('');
+}
+
 describe('expressions', () => {
   describe('timeframes', () => {
     const timeframes = [
@@ -144,6 +168,43 @@ describe('expressions', () => {
         'Cannot measure from date to timestamp'
       );
     });
+    test('compare to truncation uses straight comparison', () => {
+      const compare = expr`ad = ad.quarter`;
+      expect(compare).toTranslate();
+      const compare_expr = compare.translator.generated().value;
+      expect(exprToString(compare_expr)).toEqual('A=B');
+    });
+    test('compare to granular result expression uses straight comparison', () => {
+      const compare = expr`ad = ad.quarter + 1`;
+      expect(compare).toTranslate();
+      const compare_expr = compare.translator.generated().value;
+      expect(exprToString(compare_expr)).toEqual('A=B');
+    });
+    test('apply granular-truncation uses range', () => {
+      const compare = expr`ad ? ad.quarter`;
+      expect(compare).toTranslate();
+      const compare_expr = compare.translator.generated().value;
+      expect(exprToString(compare_expr)).toEqual('(A>=B)and(A<C)');
+    });
+    test('apply granular-literal alternation uses range', () => {
+      const compare = expr`ad ? @2020 | @2022`;
+      expect(compare).toTranslate();
+      const compare_expr = compare.translator.generated().value;
+      expect(exprToString(compare_expr)).toEqual(
+        '((A>=B)and(A<C))or((A>=D)and(A<E))'
+      );
+    });
+    // this should use range, but it uses = and alternations are
+    // kind of needing help so this is a placeholder for
+    // future work
+    test.skip('apply granular-result alternation uses range', () => {
+      const compare = expr`ad ? ad.year | ad.month`;
+      expect(compare).toTranslate();
+      const compare_expr = compare.translator.generated().value;
+      expect(exprToString(compare_expr)).toEqual(
+        '((A>=B)and(A<C))or((A>=D)and(A<E))'
+      );
+    });
     test('comparison promotes date literal to timestamp', () => {
       expect(expr`@2001 = ats`).toTranslate();
     });

packages/malloy/src/lang/test/test-translator.ts

@@ -511,14 +511,14 @@ export interface MarkedSource {
   translator?: TestTranslator;
 }
 
-interface HasTranslator extends MarkedSource {
-  translator: TestTranslator;
+interface HasTranslator<TT extends TestTranslator> extends MarkedSource {
+  translator: TT;
 }
 
 export function expr(
   unmarked: TemplateStringsArray,
   ...marked: string[]
-): HasTranslator {
+): HasTranslator<BetaExpression> {
   const ms = markSource(unmarked, ...marked);
   return {
     ...ms,
@@ -529,7 +529,7 @@ export function expr(
 export function model(
   unmarked: TemplateStringsArray,
   ...marked: string[]
-): HasTranslator {
+): HasTranslator<TestTranslator> {
   const ms = markSource(unmarked, ...marked);
   return {
     ...ms,
@@ -545,7 +545,7 @@ export function makeModelFunc(options: {
   return function model(
     unmarked: TemplateStringsArray,
     ...marked: string[]
-  ): HasTranslator {
+  ): HasTranslator<TestTranslator> {
     const ms = markSource(unmarked, ...marked);
     return {
       ...ms,