make = NULL and != NULL warn

demo/malloy-duckdb-wasm/README.md

@@ -73,7 +73,7 @@ source: airports is duckdb,table('airports.parquet') extend {
     heliport_count is airport_count {where: fac_type = 'HELIPORT'}
 
   view: by_state is {
-    where: state != null
+    where: state is not null
     group_by: state
     aggregate: airport_count
   }

packages/malloy-db-snowflake/src/snowflake_connection.spec.ts

@@ -66,7 +66,7 @@ describe('db:Snowflake', () => {
       .loadModel("source: aircraft is snowflake.table('malloytest.aircraft')")
       .loadQuery(
         `run:  aircraft -> {
-        where: state != null
+        where: state is not null
         aggregate: cnt is count()
         group_by:  state}`
       )

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

@@ -93,10 +93,6 @@ export class ExprEquality extends ExprCompare {
     super(left, op, right);
   }
 
-  getExpression(fs: FieldSpace): ExprValue {
-    return this.right.apply(fs, this.op, this.left, true);
-  }
-
   apply(
     fs: FieldSpace,
     op: BinaryMalloyOperator,

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

@@ -21,15 +21,34 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
+import {BinaryMalloyOperator, FieldSpace} from '..';
 import {ExprValue, literalExprValue} from '../types/expr-value';
 import {ExpressionDef} from '../types/expression-def';
 
 export class ExprNULL extends ExpressionDef {
   elementType = 'NULL';
+
   getExpression(): ExprValue {
     return literalExprValue({
       dataType: {type: 'null'},
       value: {node: 'null'},
     });
   }
+
+  apply(
+    fs: FieldSpace,
+    op: BinaryMalloyOperator,
+    left: ExpressionDef
+  ): ExprValue {
+    if (op === '!=' || op === '=') {
+      const expr = left.getExpression(fs);
+      expr.type = 'boolean';
+      expr.value = {
+        node: op === '=' ? 'is-null' : 'is-not-null',
+        e: expr.value,
+      };
+      return expr;
+    }
+    return super.apply(fs, op, left, true);
+  }
 }

packages/malloy/src/lang/ast/types/expression-def.ts

@@ -300,25 +300,6 @@ function regexEqual(left: ExprValue, right: ExprValue): Expr | undefined {
   return undefined;
 }
 
-function nullCompare(
-  left: ExprValue,
-  op: string,
-  right: ExprValue
-): Expr | undefined {
-  const not = op === '!=' || op === '!~';
-  const nullCmp = not ? 'is-not-null' : 'is-null';
-  if (left.type === 'null' || right.type === 'null') {
-    if (left.type !== 'null') {
-      return {node: nullCmp, e: left.value};
-    }
-    if (right.type !== 'null') {
-      return {node: nullCmp, e: right.value};
-    }
-    return {node: not ? 'false' : 'true'};
-  }
-  return undefined;
-}
-
 function equality(
   fs: FieldSpace,
   left: ExpressionDef,
@@ -349,30 +330,21 @@ function equality(
     kids: {left: lhs.value, right: rhs.value},
   };
 
-  if (lhs.type !== 'error' && rhs.type !== 'error') {
-    switch (op) {
-      case '~':
-      case '!~': {
-        if (lhs.type !== 'string' || rhs.type !== 'string') {
-          let regexCmp = regexEqual(lhs, rhs);
-          if (regexCmp) {
-            if (op[0] === '!') {
-              regexCmp = {node: 'not', e: {...regexCmp}};
-            }
-          } else {
-            throw new TypeMismatch(
-              "Incompatible types for match('~') operator"
-            );
-          }
-          value = regexCmp;
+  if (
+    lhs.type !== 'error' &&
+    rhs.type !== 'error' &&
+    (op === '~' || op === '!~')
+  ) {
+    if (lhs.type !== 'string' || rhs.type !== 'string') {
+      let regexCmp = regexEqual(lhs, rhs);
+      if (regexCmp) {
+        if (op[0] === '!') {
+          regexCmp = {node: 'not', e: {...regexCmp}};
         }
-        break;
-      }
-      case '=':
-      case '!=': {
-        value = nullCompare(lhs, op, rhs) ?? value;
-        break;
+      } else {
+        throw new TypeMismatch("Incompatible types for match('~') operator");
       }
+      value = regexCmp;
     }
   }
 

packages/malloy/src/lang/malloy-to-ast.ts

@@ -1348,6 +1348,24 @@ export class MalloyToAST
     const left = this.getFieldExpr(pcx.fieldExpr(0));
     const right = this.getFieldExpr(pcx.fieldExpr(1));
     if (ast.isEquality(op)) {
+      const wholeRange = this.parseInfo.rangeFromContext(pcx);
+      if (right instanceof ast.ExprNULL) {
+        if (op === '=') {
+          this.warnWithReplacement(
+            'sql-is-null',
+            "Use IS NULL instead of '= null'",
+            wholeRange,
+            `${this.getSourceCode(pcx.fieldExpr(0))} is null`
+          );
+        } else if (op === '!=') {
+          this.warnWithReplacement(
+            'sql-is-not-null',
+            "Use IS NOT NULL instead of '!= null'",
+            wholeRange,
+            `${this.getSourceCode(pcx.fieldExpr(0))} is not null`
+          );
+        }
+      }
       return this.astAt(new ast.ExprEquality(left, op, right), pcx);
     } else if (ast.isComparison(op)) {
       return this.astAt(new ast.ExprCompare(left, op, right), pcx);

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

@@ -1292,7 +1292,7 @@ describe('expressions', () => {
     ['astr', 'string'],
     ['abool', 'boolean'],
   ])('Can compare field %s (type %s) to NULL', (name, _datatype) => {
-    expect(expr`${name} = NULL`).toTranslate();
+    expect(expr`${name} IS NULL`).toTranslate();
   });
 });
 describe('alternations as in', () => {
@@ -1353,7 +1353,7 @@ describe('sql native fields in schema', () => {
   });
   test('sql native reference can be compared to NULL', () => {
     const uModel = new TestTranslator(
-      'run: a->{ where: aun != NULL; select: * }'
+      'run: a->{ where: aun !IS NULL; select: * }'
     );
     expect(uModel).toTranslate();
   });

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

@@ -71,7 +71,7 @@ describe('parameters', () => {
     expect(`
       ##! experimental.parameters
       source: ab_new(param is null::string) is ab extend {
-        where: param = null
+        where: param is null
       }
         run: ab_new(param is "foo") -> { select: * } -> { select: * }
     `).toTranslate();

test/src/databases/all/composite_sources.spec.ts

@@ -397,7 +397,7 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => {
         run: compose(
           state_facts extend { dimension: bar is 1 },
           state_facts
-        ) -> { index: bar } -> { group_by: fieldName; where: fieldName != null }
+        ) -> { index: bar } -> { group_by: fieldName; where: fieldName is not null }
       `).malloyResultMatches(runtime, {fieldName: 'bar'});
     });
   });

test/src/databases/all/expr.spec.ts

@@ -234,7 +234,7 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => {
       run: aircraft->{
         top: 10
         order_by: 1
-        where: region != NULL
+        where: region is not null
         group_by: region
         nest: by_state is {
           top: 10
@@ -823,7 +823,7 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => {
           SELECT '' as ${q`null_value`}, '' as ${q`string_value`}
           UNION ALL SELECT null, 'correct'
       """) -> {
-        where: null_value = null
+        where: null_value is null
         select:
           found_null is  null_value ?? 'correct',
           else_pass is string_value ?? 'incorrect'

test/src/databases/all/functions.spec.ts

@@ -642,7 +642,7 @@ expressionModels.forEach((x, databaseName) => {
           `
           run: aircraft -> {
             group_by: state
-            where: state != null
+            where: state is not null
             nest: by_county is {
               limit: 2
               group_by: county
@@ -680,7 +680,7 @@ expressionModels.forEach((x, databaseName) => {
           `
           run: airports extend { measure: airport_count is count() } -> {
             group_by: state
-            where: state != null
+            where: state is not null
             calculate: prev_airport_count is lag(airport_count)
           }`
         )

test/src/databases/all/nomodel.spec.ts

@@ -82,7 +82,7 @@ runtimes.runtimeMap.forEach((runtime, databaseName) => {
   it(`bug 151 which used to throw unknown dialect is still fixed- ${databaseName}`, async () => {
     await expect(`
       query: q is ${databaseName}.table('malloytest.aircraft')->{
-        where: state != null
+        where: state is not null
         group_by: state
       }
       run: q extend {
@@ -1035,7 +1035,7 @@ SELECT row_to_json(finalStage) as row FROM __stage0 AS finalStage`);
           ${splitFN!(q`city`, ' ')} as ${q`words`}
         FROM ${rootDbPath(databaseName)}malloytest.aircraft
       """) -> {
-        where: words.value != null
+        where: words.value is not null
         group_by: words.value
         aggregate: c is count()
       }
@@ -1089,7 +1089,7 @@ SELECT row_to_json(finalStage) as row FROM __stage0 AS finalStage`);
     await expect(`
         source: ga_sample is ${databaseName}.table('malloytest.ga_sample')
         run: ga_sample -> {
-          where: hits.product.productBrand != null
+          where: hits.product.productBrand is not null
           group_by:
             hits.product.productBrand
             hits.product.productSKU
@@ -1124,16 +1124,16 @@ SELECT row_to_json(finalStage) as row FROM __stage0 AS finalStage`);
         .loadQuery(
           `
         run: ${databaseName}.table('malloytest.airports') -> {
-          where: faa_region = null
+          where: faa_region is null
           group_by: faa_region
           aggregate: airport_count is count()
           nest: by_state is {
-            where: state != null
+            where: state is not null
             group_by: state
             aggregate: airport_count is count()
           }
           nest: by_state1 is {
-            where: state != null
+            where: state is not null
             group_by: state
             aggregate: airport_count is count()
             limit: 1

test/src/databases/all/parameters.spec.ts

@@ -320,7 +320,7 @@ runtimes.runtimeMap.forEach((runtime, databaseName) => {
         param::string is null,
         state_filter::string is "CA"
       ) is ${databaseName}.table('malloytest.state_facts') extend {
-        where: param = null and state = state_filter
+        where: param is null and state = state_filter
       }
       run: state_facts -> { group_by: state }
     `).malloyResultMatches(runtime, {state: 'CA'});

test/src/databases/bigquery/malloy_query.spec.ts

@@ -486,7 +486,7 @@ describe('BigQuery expression tests', () => {
         }
       }
       -> {
-        where: state.code.code != null
+        where: state.code.code is not null
         group_by: state.code.code
       }
     `
@@ -639,7 +639,7 @@ describe('airport_tests', () => {
       `
       run: airports-> {
         nest: zero is {
-          nest: by_faa_region_i is { where: county ~'I%' and  state != NULL
+          nest: by_faa_region_i is { where: county ~'I%' and  state is not null
             group_by: faa_region
             aggregate: airport_count
             nest: by_state is {

test/src/databases/bigquery/nested_source_table.spec.ts

@@ -154,7 +154,7 @@ describe.each(runtimes.runtimeList)(
     test(`search_index - ${databaseName}`, async () => {
       await expect(`
         run: ga_sessions->search_index -> {
-          where: fieldName != null
+          where: fieldName is not null
           select: *
           order_by: fieldName, weight desc
           limit: 10

test/src/databases/duckdb/nested_source_table.spec.ts

@@ -154,7 +154,7 @@ describe.each(runtimes.runtimeList)(
     test(`search_index - ${databaseName}`, async () => {
       await expect(`
         run: ga_sessions->search_index -> {
-          where: fieldName != null
+          where: fieldName is not null
           select: *
           order_by: fieldName, weight desc
           limit: 10

test/src/databases/duckdb/streaming.spec.ts

@@ -56,15 +56,15 @@ function modelText(databaseName: string) {
   }
 
   view: by_county is {
-    where: county != null
+    where: county is not null
     group_by: county
     aggregate: airport_count
     limit: 2
     order_by: airport_count desc, county desc
   }
 
   view: by_state is {
-    where: state != null
+    where: state is not null
     group_by: state
     aggregate: airport_count
     limit: 2