From ff6cf80bda45912faaeaffd478a874409ba3dbe7 Mon Sep 17 00:00:00 2001
From: Michael Toy <66150587+mtoy-googly-moogly@users.noreply.github.com>
Date: Sat, 28 Dec 2024 09:19:26 -0800
Subject: [PATCH] add tests

---
 packages/malloy/src/doc/null-safe.md          | 41 +++++----
 .../src/databases/all/compound-atomic.spec.ts | 12 +++
 test/src/databases/all/expr.spec.ts           | 83 +++++++++++++++++++
 3 files changed, 120 insertions(+), 16 deletions(-)

diff --git a/packages/malloy/src/doc/null-safe.md b/packages/malloy/src/doc/null-safe.md
index b8ce2b9e27..6952f0d300 100644
--- a/packages/malloy/src/doc/null-safe.md
+++ b/packages/malloy/src/doc/null-safe.md
@@ -1,29 +1,38 @@
 # NULL fields are a problem
-Because any field might be null, any expression might have NULL values.
+Because any field might be null, any expression might have NULL values. NULL in SQL is infecting,
+so any expression which has a NULL in it returns a NULL value.
 
-SQL basically says that any expression involving null values results in null value.
+For booleans, especially for boolean dimensions, a very common pattern in Malloy, you end
+up creating expressions which contain nonsense data because NOT null is null, or because
+two values which are both NULL are NULL when compared which is false-y not equal.
 
-For booleans, the creates a problem where "condition" and "not condition" cannot
-be assumed to cover all possibilities (because they might both be NULL)
+## The Malloy Null Safe Truth Tables
 
+### Boolean NOT
 
-## Null Safe Not
+| Expression | x=null | x=true | x=false
+| ---- | ---- | ----- | ---- |
+| `not x` | `true` | `false` | `true` |
 
-As an experiment, the following negation operators are all protected against NULL
-values.
+### Non null to nullable
 
-It is an open question if this is a good idea or not, but this has been how Malloy
-works for a while, so to err on the side of safety, I am simply writing down
-what happens now and moving on
+| Expression | x=null |
+| ---- | ---- |
+| `x = 0` | `false` |
+| `x != 0` | `true` |
+| `x ~ 'a'` | `false` |
+| `x !~ 'a'` | `true` |
+| `x ~ r'a'` | `false` |
+| `x !~ r'a'` | `true` |
 
-This transformation used to happen in the compiler, but at this writing,
-it now happens it code generation time.
+### Compare two nullable
 
-| Expression | Null Safe Version |
+| Expression | x=null, y=null |
 | ---- | ---- |
-| `not x` | `coalesce(not x, true)` |
-| `a != b` | `coalesce(a != b, true)` |
-| `a !~ b` | `coalesce(a != b, true)` |
+| `x = y` | `true` |
+| `x != y` | `false` |
+| `x ~ y` | `true` |
+| `x !~ y` | `false` |
 
 ## Null Safe Functions
 
diff --git a/test/src/databases/all/compound-atomic.spec.ts b/test/src/databases/all/compound-atomic.spec.ts
index 7e6aa6520b..40ec3e4645 100644
--- a/test/src/databases/all/compound-atomic.spec.ts
+++ b/test/src/databases/all/compound-atomic.spec.ts
@@ -529,6 +529,18 @@ describe.each(runtimes.runtimeList)(
           {val: 1, name: 'uno'},
         ]);
       });
+      test('group_by repeated record', async () => {
+        await expect(`
+          run: ${conName}.sql(""" ${selectAB('ab')} """) -> { group_by: ab }
+        `).malloyResultMatches(runtime, {ab: ab_eq});
+      });
+      // test for https://github.com/malloydata/malloy/issues/2065
+      test.skip('nest a groupoed repeated record', async () => {
+        await expect(`
+          run: ${conName}.sql(""" ${selectAB('ab')} """)
+          -> { nest: gab is {group_by: ab} }
+        `).malloyResultMatches(runtime, {ab: ab_eq});
+      });
     });
   }
 );
diff --git a/test/src/databases/all/expr.spec.ts b/test/src/databases/all/expr.spec.ts
index 7c0b448018..f9c4cb19d4 100644
--- a/test/src/databases/all/expr.spec.ts
+++ b/test/src/databases/all/expr.spec.ts
@@ -850,6 +850,89 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => {
       no_paren: booleanResult(false, databaseName),
     });
   });
+
+  describe('null safe booleans', () => {
+    // Snowflake makes us do this ...
+    const y = runtime.dialect.sqlMaybeQuoteIdentifier('y');
+    const a = runtime.dialect.sqlMaybeQuoteIdentifier('a');
+    const b = runtime.dialect.sqlMaybeQuoteIdentifier('b');
+    const tf = runtime.dialect.sqlMaybeQuoteIdentifier('tf');
+    const x = runtime.dialect.sqlMaybeQuoteIdentifier('x');
+    // Create nulls of a few types
+    const nulls = `${databaseName}.sql("""
+      SELECT
+        false as has_nulls,
+          1 as ${x}, 2 as ${y},
+          'a' as ${a}, 'b' as ${b},
+          true as ${tf}
+      UNION ALL SELECT
+        true,
+          null, null, null, null, null
+    """) extend { where: has_nulls }`;
+
+    it('select boolean', async () => {
+      await expect(`run: ${nulls} -> {
+        select:
+          null_boolean is tf
+      }`).malloyResultMatches(runtime, {null_boolean: null});
+    });
+    it('not boolean', async () => {
+      await expect(`run: ${nulls} -> {
+        select:
+          not_null_boolean is not tf
+      }`).malloyResultMatches(runtime, {not_null_boolean: true});
+    });
+    it('numeric = non-null to null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: val_eq_null is x = 9 }`
+      ).malloyResultMatches(runtime, {val_eq_null: false});
+    });
+    it('numeric != non-null to null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: val_ne_null is x != 9 }`
+      ).malloyResultMatches(runtime, {val_ne_null: true});
+    });
+    it('string ~ non-null to null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: val_eq_null is a ~ 'z' }`
+      ).malloyResultMatches(runtime, {val_eq_null: false});
+    });
+    it('string !~ non-null to null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: val_ne_null is a !~ 'z' }`
+      ).malloyResultMatches(runtime, {val_ne_null: true});
+    });
+    it('regex ~ non-null to null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: val_eq_null is a ~ r'z' }`
+      ).malloyResultMatches(runtime, {val_eq_null: false});
+    });
+    it('regex !~ non-null to null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: val_ne_null is a !~ r'z' }`
+      ).malloyResultMatches(runtime, {val_ne_null: true});
+    });
+    it('numeric = null-to-null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: null_eq_null is x = y }`
+      ).malloyResultMatches(runtime, {null_eq_null: true});
+    });
+    it('numeric != null-to-null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: null_ne_null is x != y }`
+      ).malloyResultMatches(runtime, {null_ne_null: false});
+    });
+    it('string ~ null-to-null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: null_eq_null is a ~ b }`
+      ).malloyResultMatches(runtime, {null_eq_null: true});
+    });
+    it('string !~ null-to-null', async () => {
+      await expect(
+        `run: ${nulls} -> { select: null_ne_null is a !~ b }`
+      ).malloyResultMatches(runtime, {null_ne_null: false});
+    });
+  });
 });
 
 afterAll(async () => {