Skip to content

Commit

Permalink
add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
mtoy-googly-moogly committed Dec 28, 2024
1 parent be9f2bb commit ff6cf80
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 16 deletions.
41 changes: 25 additions & 16 deletions packages/malloy/src/doc/null-safe.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
12 changes: 12 additions & 0 deletions test/src/databases/all/compound-atomic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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});
});
});
}
);
Expand Down
83 changes: 83 additions & 0 deletions test/src/databases/all/expr.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down

0 comments on commit ff6cf80

Please sign in to comment.