A boolean is coerced between condition and value in the compiler

packages/malloy-db-sqlserver/README.md

@@ -49,7 +49,7 @@ Cancelling a query uses the driver, so no `VIEW SERVER STATE` or `ALTER ANY CONN
 
 ## What works
 
-Sources from tables (`table('schema.table')`, `[bracketed]` names, three-part `db.schema.table`) and SQL blocks; `select`, `group_by`, `aggregate`, `where`, `having`, `order_by`, `limit`; expressions as dimensions; joins with symmetric aggregates across a fan-out; multi-stage pipelines and a query used as a source; dates, timestamps, truncation, extraction, intervals, and `timezone:` with IANA names through `AT TIME ZONE`; the standard function library in its T-SQL spellings.
+Sources from tables (`table('schema.table')`, `[bracketed]` names, three-part `db.schema.table`) and SQL blocks; `select`, `group_by`, `aggregate`, `where`, `having`, `order_by`, `limit`; expressions as dimensions; joins with symmetric aggregates across a fan-out; multi-stage pipelines and a query used as a source; dates, timestamps, truncation, extraction, intervals, and `timezone:` with IANA names through `AT TIME ZONE`; the standard function library in its T-SQL spellings; a boolean as a condition: a comparison, a declared boolean, `not`, `!=`, `!~` or a boolean function in `where:`, `having:`, a join `on:`, a filtered aggregate and `pick … when`.
 
 ## What does not work yet
 
@@ -59,9 +59,7 @@ Each of these is a later change, not a promise.
 |---|---|
 | `nest:` | Refused at translation (`supportsNesting` is false) |
 | Arrays and records, in table data or as literals | Declared unsupported; a JSON column is a string |
-| A boolean where SQL expects a condition: a boolean dimension in `where:`, `having:` or `pick … when`; a `bit` column in `where:` | Server error (4145). A comparison, `starts_with`, `ends_with`, `is_nan` or `is_inf` written there directly works, and a boolean **declared** as a dimension or measure is a `1`/`0` value (the translator's CASE for `booleanType: 'none'`) that groups and selects but is not a condition |
-| `x::boolean` | Compile error: there is no boolean to cast to |
-| `not x` on any boolean, `x !~ 'pattern'` | Server error (4145): the compiler writes `COALESCE(NOT x, TRUE)` and `COALESCE(x LIKE p, true)`, and a condition is not a value `COALESCE` can take. `x != y` works; `not (x = y)` does not |
+| A boolean where SQL expects a value: a declared boolean or a comparison in `select:`, `group_by:`, `aggregate:` or `index:`, as a function argument, in `??`, `is null`, `=` or a cast, or as the result of a `pick` | Compile error at the expression, naming the value to write instead, `pick 'yes' when is_late else 'no'`. A boolean is a condition on this dialect and never a value; `select: *` leaves a boolean out with a warning |
 | `all()`, `exclude()` | Compile error: ungrouped aggregates share the nest machinery |
 | `x ~ r'...'` | Error when SQL is generated: no regular expressions before SQL Server 2025 |
 | `regexp_extract`, `replace` with a regular expression | Server error: the functions arrive in SQL Server 2025 |
@@ -81,6 +79,7 @@ Every Malloy query is one T-SQL batch: the session settings `SET DATEFIRST 7`, `
 - A `datetimeoffset` is a Malloy `timestamptz`, an instant. Its clock is read at UTC, or in the query's `timezone:`, not at the offset the value was written in, so `t.day` is the same for `12:34:56 -05:00` as for `17:34:56 +00:00`.
 - Every string literal is written `N'...'`, so text outside the database's code page compares as itself.
 - The default collation compares case-insensitively; Malloy does not change that.
+- A boolean is a condition, never a value: `where: is_late` writes the comparison `is_late` was declared as. Malloy's `not` is true of NULL and `x != y` is true when either side is NULL, as on every dialect, written as `CASE … END = 0` tests rather than `COALESCE`, which has no boolean to return here.
 - `bit` is an integer, read as 0 or 1. `uniqueidentifier`, `text`, `ntext`, `time`, `binary`, `varbinary`, `xml`, `geography` and the other types the map does not name are `sql native`, to be passed through or cast in a `sql()` dimension.
 - A `bigint` arrives as the driver's text and is read as a number, so a value above 2^53 loses precision.
 - The connection's digest, which names the tables it may persist and the cache it may share, is built from the server, port, instance, database, authentication kind and the configured principal: the `user`, `domain\user`, or `clientId`. An ambient identity, `azure-default` or `azure-msi` with no `clientId`, is unknown until the first token request, so two such connections to one database share a digest. To tell them apart, set `clientId` (a user-assigned identity) or use `azure-service-principal`.

packages/malloy/src/dialect/dialect.spec.ts

@@ -5,13 +5,28 @@
 
 import {MySQLDialect} from './mysql';
 import {SQLServerDialect} from './sqlserver';
+import {StandardSQLDialect} from './standardsql';
 
 describe('Dialect booleans', () => {
+  const noBooleans = new SQLServerDialect();
+  const booleans = new StandardSQLDialect();
+
   test('a dialect with no boolean type spells both literals as conditions', () => {
-    const noBooleans = new SQLServerDialect();
     expect(noBooleans.sqlBoolean(true)).toBe('(1=1)');
     expect(noBooleans.sqlBoolean(false)).toBe('(1=0)');
   });
+
+  test('not, and a comparison true on NULL, keep COALESCE where there are booleans', () => {
+    expect(booleans.sqlNot('c')).toBe('COALESCE(NOT c,TRUE)');
+    expect(booleans.sqlNullIsTrue('c')).toBe('COALESCE(c,true)');
+  });
+
+  test('not, and a comparison true on NULL, are written without a boolean value', () => {
+    expect(noBooleans.sqlNot('c')).toBe('(CASE WHEN c THEN 1 ELSE 0 END = 0)');
+    expect(noBooleans.sqlNullIsTrue('c')).toBe(
+      '(CASE WHEN NOT (c) THEN 1 ELSE 0 END = 0)'
+    );
+  });
 });
 
 describe('Dialect ORDER BY and row limit', () => {

packages/malloy/src/dialect/dialect.ts

@@ -1241,6 +1241,49 @@ export abstract class Dialect {
     return bv ? 'true' : 'false';
   }
 
+  /**
+   * Malloy's `not`: of NULL, true. A dialect with booleanType 'none' has no
+   * boolean value for COALESCE to return, so the test is written as a
+   * condition.
+   */
+  sqlNot(condition: string): string {
+    if (this.booleanType === 'none') {
+      return `(CASE WHEN ${condition} THEN 1 ELSE 0 END = 0)`;
+    }
+    return `COALESCE(NOT ${condition},TRUE)`;
+  }
+
+  /**
+   * A condition Malloy defines as true when SQL says NULL: `!=` and `!~`
+   */
+  sqlNullIsTrue(condition: string): string {
+    if (this.booleanType === 'none') {
+      return `(CASE WHEN NOT (${condition}) THEN 1 ELSE 0 END = 0)`;
+    }
+    return `COALESCE(${condition},true)`;
+  }
+
+  /**
+   * A condition, where a value is wanted. Without a boolean value the
+   * condition becomes 1, 0, or NULL when it is unknown.
+   */
+  sqlConditionAsValue(condition: string): string {
+    if (this.booleanType === 'none') {
+      return `CASE WHEN ${condition} THEN 1 WHEN NOT (${condition}) THEN 0 END`;
+    }
+    return condition;
+  }
+
+  /**
+   * A value, where a condition is wanted: the value a condition became.
+   */
+  sqlValueAsCondition(value: string): string {
+    if (this.booleanType === 'none') {
+      return `(${value} = 1)`;
+    }
+    return value;
+  }
+
   /**
    * What a boolean value looks like in a query result
    */

packages/malloy/src/doc/boolean-none.md

@@ -43,16 +43,16 @@ is compiled for a `'none'` dialect.
 - A boolean field in a query's output: `select:`, `group_by:`, `aggregate:`,
   and any other place a field becomes a result column.
 - `is null` and `is not null` with a boolean operand.
-- `=` and `!=` with two boolean operands.
+- Any comparison (`=`, `!=`, `<`, `<=`, `>`, `>=`) with a boolean operand
+  (`is_small = 1`, `size > is_small`, `is_small ? > false`), written plainly,
+  as a partial or in an alternation.
 - `??` with a boolean operand.
 - A `pick` or `case` whose result is a boolean.
 - A boolean filter expression (`x ~ f'true'`).
 - A boolean as a function argument.
 - A cast to or from boolean (`x::boolean`, `is_small::string`).
-- An alternation with a boolean on the left (`is_small ? true | false`).
 
-The error suggests making the value explicit with a `pick`, as in the example
-above.
+Each error says what to write instead.
 
 ## `select: *`
 

packages/malloy/src/lang/ast/expressions/boolean-value.ts

@@ -0,0 +1,60 @@
+/*
+ * Copyright Contributors to the Malloy project
+ * SPDX-License-Identifier: MIT
+ */
+
+import type {ExprValue} from '../types/expr-value';
+import type {FieldSpace} from '../types/field-space';
+import type {MalloyElement} from '../types/malloy-element';
+
+/**
+ * On a dialect with booleanType 'none' a boolean is a condition and never a
+ * value. When `type` is a boolean about to be used as a value, logs an error
+ * at `logTo` and returns an error-typed value for the caller to return in
+ * place of what it was building, so the enclosing expressions see an error
+ * and stay quiet. Otherwise returns undefined. `where` says how the boolean
+ * was used, `hint` what to write instead.
+ */
+export function booleanValueError(
+  fs: FieldSpace,
+  type: {type: string} | undefined,
+  where: string,
+  logTo: MalloyElement,
+  hint: string,
+  evenWhenCoerced = false
+): ExprValue | undefined {
+  const dialect = fs.dialectObj();
+  if (type?.type !== 'boolean' || dialect?.booleanType !== 'none') {
+    return undefined;
+  }
+  // The compiler coerces a boolean to a value in the boolean_coercion
+  // experiment, so no use of one is refused, unless there is no value the
+  // use could coerce to
+  if (!evenWhenCoerced && logTo.inExperiment('boolean_coercion', true)) {
+    return undefined;
+  }
+  return logTo.loggedErrorExpr('boolean-value-unsupported', {
+    dialect: dialect.name,
+    where,
+    hint,
+  });
+}
+
+/** The hint where a boolean's unknown state is examined: `is null`, `??` */
+export const CONDITION_HINT =
+  'test the condition itself; unknown already tests as false';
+
+/** The hint for a boolean compared with anything */
+export const COMPARE_HINT = 'test the condition itself, or its not';
+
+/** The hint where a number would do: a function argument, a cast */
+export const NUMBER_HINT = 'write the value, pick 1 when … else 0';
+
+/** The hint for a boolean as a result column, from the source text it came from */
+export function resultColumnHint(src: string): string {
+  return `write the value, pick 'yes' when ${src} else 'no'`;
+}
+
+/** The hint for a `pick` or `case` whose result is a boolean */
+export const CHOICE_HINT =
+  "write values, pick 'yes' when … else 'no', or the condition itself with and, or and not";

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

@@ -10,6 +10,7 @@ import type {FieldSpace} from '../types/field-space';
 import {MalloyElement} from '../types/malloy-element';
 import * as TDU from '../typedesc-utils';
 import type {CaseExpr} from '../../../model';
+import {booleanValueError, CHOICE_HINT} from './boolean-value';
 
 interface Choice {
   then: ExprValue;
@@ -45,6 +46,14 @@ export class Case extends ExpressionDef {
     let value: ExprValue | undefined = undefined;
     if (this.value) {
       const v = this.value.getExpression(fs);
+      const noSubject = booleanValueError(
+        fs,
+        v,
+        'as the subject of a case',
+        this.value,
+        'write the conditions, case when … then …'
+      );
+      if (noSubject) return noSubject;
       dependents.push(v);
       resultExpr.kids.caseValue = v.value;
       value = v;
@@ -94,6 +103,14 @@ export class Case extends ExpressionDef {
       dependents.push(elseValue);
       resultExpr.kids.caseElse = elseValue.value;
     }
+    const noCase = booleanValueError(
+      fs,
+      returnType,
+      'as the result of a case',
+      this,
+      CHOICE_HINT
+    );
+    if (noCase) return noCase;
     return computedExprValue({
       value: resultExpr,
       dataType: returnType ? TDU.atomicDef(returnType) : {type: 'null'},

packages/malloy/src/lang/ast/expressions/expr-alternation-tree.ts

@@ -9,6 +9,7 @@ import type {FieldSpace} from '../types/field-space';
 import {ATNodeType, ExpressionDef} from '../types/expression-def';
 import type {BinaryMalloyOperator} from '../types/binary_operators';
 import {isEquality} from '../types/binary_operators';
+import {booleanValueError, COMPARE_HINT} from './boolean-value';
 
 /**
  * Return a flattened version of an alternation tree, if the tree is
@@ -70,6 +71,21 @@ export class ExprAlternationTree extends ExpressionDef {
       if (inList.length > 0 && (applyOp === '=' || applyOp === '!=')) {
         const isIn = expr.getExpression(fs);
         const values = inList.map(v => v.getExpression(fs));
+        // A boolean on either side of the comparison, the tested value or
+        // any of the choices, is a boolean compared with something.
+        for (const [el, val] of [
+          [expr, isIn] as const,
+          ...inList.map((el, i) => [el, values[i]] as const),
+        ]) {
+          const noIn = booleanValueError(
+            fs,
+            val,
+            `compared with ${applyOp}`,
+            el,
+            COMPARE_HINT
+          );
+          if (noIn) return noIn;
+        }
         return computedExprValue({
           dataType: {type: 'boolean'},
           value: {

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

@@ -9,6 +9,7 @@ import type {ExprValue} from '../types/expr-value';
 import {computedExprValue} from '../types/expr-value';
 import {ExpressionDef} from '../types/expression-def';
 import type {FieldSpace} from '../types/field-space';
+import {booleanValueError, NUMBER_HINT} from './boolean-value';
 
 export class ExprCast extends ExpressionDef {
   elementType = 'cast';
@@ -46,6 +47,18 @@ export class ExprCast extends ExpressionDef {
         }
       }
     }
+    const noCast =
+      booleanValueError(fs, expr, 'in a cast', this, NUMBER_HINT) ||
+      // A cast makes a value, and there is no boolean value to make
+      booleanValueError(
+        fs,
+        dataType,
+        'as the type of a cast',
+        this,
+        'write a comparison',
+        true
+      );
+    if (noCast) return noCast;
     return computedExprValue({
       dataType,
       value: castTo(this.castType, expr.value, expr.type, this.safe),

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

@@ -9,6 +9,7 @@ import * as TDU from '../typedesc-utils';
 import type {ExprValue} from '../types/expr-value';
 import {ExpressionDef} from '../types/expression-def';
 import type {FieldSpace} from '../types/field-space';
+import {booleanValueError, CONDITION_HINT} from './boolean-value';
 
 export class ExprCoalesce extends ExpressionDef {
   elementType = 'coalesce expression';
@@ -23,6 +24,14 @@ export class ExprCoalesce extends ExpressionDef {
   protected computeExpression(fs: FieldSpace): ExprValue {
     const maybeNull = this.expr.getExpression(fs);
     const whenNull = this.altExpr.getExpression(fs);
+    const noCoalesce = booleanValueError(
+      fs,
+      maybeNull,
+      'in ??',
+      this,
+      CONDITION_HINT
+    );
+    if (noCoalesce) return noCoalesce;
     if (maybeNull.type === 'null') {
       return whenNull;
     }

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

@@ -60,6 +60,7 @@ import {composeSQLExpr} from '../../../model/utils';
 import * as TDU from '../typedesc-utils';
 import {mergeRefSummaries} from '../../composite-source-utils';
 import type {AnyMessageCodeAndParameters} from '../../parse-log';
+import {booleanValueError, NUMBER_HINT} from './boolean-value';
 
 // Built-in functions that take a string literal of user-supplied SQL
 // and emit it directly. Gated by experimental.sql_functions in plain
@@ -154,6 +155,16 @@ export class ExprFunc extends ExpressionDef {
   ): ExprValue {
     const argExprsWithoutImplicit = this.args.map(arg => arg.getExpression(fs));
     if (this.isRaw) {
+      for (const [i, argExpr] of argExprsWithoutImplicit.entries()) {
+        const noArg = booleanValueError(
+          fs,
+          argExpr,
+          `as an argument of ${this.name}`,
+          this.args[i],
+          NUMBER_HINT
+        );
+        if (noArg) return noArg;
+      }
       const funcCall: SQLExprElement[] = [`${this.name}(`];
       argExprsWithoutImplicit.forEach((expr, i) => {
         if (i !== 0) {
@@ -237,6 +248,16 @@ export class ExprFunc extends ExpressionDef {
       nullabilityErrors,
       returnType,
     } = result;
+    for (const [i, argExpr] of argExprs.entries()) {
+      const noArg = booleanValueError(
+        fs,
+        argExpr,
+        `as an argument of ${this.name}`,
+        this.args[i - (implicitExpr ? 1 : 0)] ?? this,
+        NUMBER_HINT
+      );
+      if (noArg) return noArg;
+    }
     // Report errors for expression type mismatch
     for (const error of expressionTypeErrors) {
       const adjustedIndex = error.argIndex - (implicitExpr ? 1 : 0);
@@ -450,7 +471,13 @@ export class ExprFunc extends ExpressionDef {
           }
         }
 
-        funcCall = composeSQLExpr(expr);
+        const sqlExpr = composeSQLExpr(expr);
+        sqlExpr.authored = true;
+        // The compiler needs to know this raw SQL is a condition
+        if (func.name === 'sql_boolean') {
+          sqlExpr.typeDef = {type: 'boolean'};
+        }
+        funcCall = sqlExpr;
       }
     }
     const maxEvalSpace = mergeEvalSpaces(...argExprs.map(e => e.evalSpace));

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

@@ -3,7 +3,6 @@
  * SPDX-License-Identifier: MIT
  */
 
-import {errorFor} from '../ast-utils';
 import * as TDU from '../typedesc-utils';
 import type {ExprValue} from '../types/expr-value';
 import {ExpressionDef} from '../types/expression-def';
@@ -19,13 +18,20 @@ export class ExprMinus extends ExpressionDef {
 
   protected computeExpression(fs: FieldSpace): ExprValue {
     const expr = this.expr.getExpression(fs);
+    // An error is already logged; it stays an error so nothing else logs it
+    if (expr.type === 'error') {
+      return expr;
+    }
     if (TDU.typeIn(expr, this.legalChildTypes)) {
       return {
         ...expr,
         type: 'number',
         value: {node: 'unary-', e: expr.value},
       };
     }
-    return errorFor('negate requires number');
+    return this.expr.loggedErrorExpr(
+      'expression-type-error',
+      `'${this.elementType}' Can't use type ${TDU.inspect(expr)}`
+    );
   }
 }