diff --git a/packages/malloy-db-sqlserver/README.md b/packages/malloy-db-sqlserver/README.md index 10381fa93..35daa9d76 100644 --- a/packages/malloy-db-sqlserver/README.md +++ b/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`. diff --git a/packages/malloy/src/dialect/dialect.spec.ts b/packages/malloy/src/dialect/dialect.spec.ts index 077e22769..2dd14053f 100644 --- a/packages/malloy/src/dialect/dialect.spec.ts +++ b/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', () => { diff --git a/packages/malloy/src/dialect/dialect.ts b/packages/malloy/src/dialect/dialect.ts index bee65d1ce..8276f13e0 100644 --- a/packages/malloy/src/dialect/dialect.ts +++ b/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 */ diff --git a/packages/malloy/src/doc/boolean-none.md b/packages/malloy/src/doc/boolean-none.md index 20c4f4e05..0ab9589ed 100644 --- a/packages/malloy/src/doc/boolean-none.md +++ b/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: *` diff --git a/packages/malloy/src/lang/ast/expressions/boolean-value.ts b/packages/malloy/src/lang/ast/expressions/boolean-value.ts new file mode 100644 index 000000000..2a6a8fd34 --- /dev/null +++ b/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"; diff --git a/packages/malloy/src/lang/ast/expressions/case.ts b/packages/malloy/src/lang/ast/expressions/case.ts index 864d88adf..5c1c3b22b 100644 --- a/packages/malloy/src/lang/ast/expressions/case.ts +++ b/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'}, diff --git a/packages/malloy/src/lang/ast/expressions/expr-alternation-tree.ts b/packages/malloy/src/lang/ast/expressions/expr-alternation-tree.ts index ad5517e04..a38eb7f1c 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-alternation-tree.ts +++ b/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: { diff --git a/packages/malloy/src/lang/ast/expressions/expr-cast.ts b/packages/malloy/src/lang/ast/expressions/expr-cast.ts index af70e3840..a0be659fc 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-cast.ts +++ b/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), diff --git a/packages/malloy/src/lang/ast/expressions/expr-coalesce.ts b/packages/malloy/src/lang/ast/expressions/expr-coalesce.ts index b0787c5ca..6fb76c1fa 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-coalesce.ts +++ b/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; } diff --git a/packages/malloy/src/lang/ast/expressions/expr-func.ts b/packages/malloy/src/lang/ast/expressions/expr-func.ts index 6d984ab89..6807a88fe 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-func.ts +++ b/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)); diff --git a/packages/malloy/src/lang/ast/expressions/expr-minus.ts b/packages/malloy/src/lang/ast/expressions/expr-minus.ts index dcc6c9f8c..4a66f866c 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-minus.ts +++ b/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,6 +18,10 @@ 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, @@ -26,6 +29,9 @@ export class ExprMinus extends ExpressionDef { 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)}` + ); } } diff --git a/packages/malloy/src/lang/ast/expressions/expr-null.ts b/packages/malloy/src/lang/ast/expressions/expr-null.ts index ba7641021..76b6f2df6 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-null.ts +++ b/packages/malloy/src/lang/ast/expressions/expr-null.ts @@ -7,12 +7,21 @@ import type {BinaryMalloyOperator, FieldSpace} from '..'; import type {ExprValue} from '../types/expr-value'; import {computedExprValue, literalExprValue} from '../types/expr-value'; import {ATNodeType, ExpressionDef} from '../types/expression-def'; +import {booleanValueError, CONDITION_HINT} from './boolean-value'; function doIsNull(fs: FieldSpace, op: string, expr: ExpressionDef): ExprValue { // Build a new value rather than retyping the operand's in place: what // getExpression hands back is shared, and the operand is usually asked for // its value again after this. const nullCmp = expr.getExpression(fs); + const noTest = booleanValueError( + fs, + nullCmp, + 'in a null test', + expr, + CONDITION_HINT + ); + if (noTest) return noTest; return computedExprValue({ dataType: {type: 'boolean'}, value: { diff --git a/packages/malloy/src/lang/ast/expressions/pick-when.ts b/packages/malloy/src/lang/ast/expressions/pick-when.ts index 5e9dba3da..d027d1966 100644 --- a/packages/malloy/src/lang/ast/expressions/pick-when.ts +++ b/packages/malloy/src/lang/ast/expressions/pick-when.ts @@ -11,6 +11,7 @@ import {computedExprValue} from '../types/expr-value'; import {ExpressionDef} from '../types/expression-def'; import type {FieldSpace} from '../types/field-space'; import {MalloyElement} from '../types/malloy-element'; +import {booleanValueError, CHOICE_HINT} from './boolean-value'; interface Choice { pick: ExprValue; @@ -110,6 +111,14 @@ export class Pick extends ExpressionDef { 'Pick statments cannot have filter expression values' ); } + const noPick = booleanValueError( + fs, + definedReturnType, + 'as the result of a pick', + this, + CHOICE_HINT + ); + if (noPick) return noPick; caseValue.kids.caseElse = defaultVal.value; return computedExprValue({ dataType: definedReturnType, @@ -186,6 +195,14 @@ export class Pick extends ExpressionDef { 'Pick statments cannot have filter expression values' ); } + const noPick = booleanValueError( + fs, + definedReturnType, + 'as the result of a pick', + this, + CHOICE_HINT + ); + if (noPick) return noPick; pick.kids.caseElse = defVal.value; return computedExprValue({ dataType: definedReturnType, diff --git a/packages/malloy/src/lang/ast/field-space/index-field-space.ts b/packages/malloy/src/lang/ast/field-space/index-field-space.ts index e9108fc9d..48da8258c 100644 --- a/packages/malloy/src/lang/ast/field-space/index-field-space.ts +++ b/packages/malloy/src/lang/ast/field-space/index-field-space.ts @@ -13,6 +13,7 @@ import type { } from '../../../model/malloy_types'; import {expressionIsScalar, TD} from '../../../model/malloy_types'; import {ErrorFactory} from '../error-factory'; +import {resultColumnHint} from '../expressions/boolean-value'; import { FieldReference, IndexFieldReference, @@ -123,12 +124,26 @@ export class IndexFieldSpace extends QueryOperationSpace { ); } else { const eTypeDesc = entry.typeDesc(); - // Don't index arrays and records if ( + // Don't index arrays and records TD.isBasicAtomic(eTypeDesc) && expressionIsScalar(eTypeDesc.expressionType) && (dialect === undefined || !dialect.ignoreInProject(name)) ) { + if ( + eTypeDesc.type === 'boolean' && + dialect?.booleanType === 'none' && + !wild.inExperiment('boolean_coercion', true) + ) { + const path = [...joinPath, name].join('.'); + wild.logWarning('wildcard-omits-boolean', { + dialect: dialect.name, + wildcard: wild.refString, + name: path, + hint: resultColumnHint(path), + }); + continue; + } expandEntries.push({name: indexName, entry}); this.expandedWild.set(indexName, { path: joinPath.concat(name), diff --git a/packages/malloy/src/lang/ast/field-space/query-spaces.ts b/packages/malloy/src/lang/ast/field-space/query-spaces.ts index 8c43b1adf..9631d1c76 100644 --- a/packages/malloy/src/lang/ast/field-space/query-spaces.ts +++ b/packages/malloy/src/lang/ast/field-space/query-spaces.ts @@ -26,6 +26,7 @@ import type { } from '../../parse-log'; import {mergeRefSummaries} from '../../composite-source-utils'; import {ErrorFactory} from '../error-factory'; +import {resultColumnHint} from '../expressions/boolean-value'; import {ReferenceField} from './reference-field'; import {RefineFromSpaceField} from './refine-from-space-field'; @@ -212,6 +213,20 @@ export abstract class QueryOperationSpace model.expressionIsScalar(eType.expressionType) && (dialect === undefined || !dialect.ignoreInProject(name)) ) { + if ( + eType.type === 'boolean' && + dialect?.booleanType === 'none' && + !wild.inExperiment('boolean_coercion', true) + ) { + const path = joinPath.concat(name).join('.'); + wild.logWarning('wildcard-omits-boolean', { + dialect: dialect.name, + wildcard: wild.refString, + name: path, + hint: resultColumnHint(path), + }); + continue; + } expandEntries.push({name, entry}); this.expandedWild.set(name, { path: joinPath.concat(name), diff --git a/packages/malloy/src/lang/ast/query-items/field-declaration.ts b/packages/malloy/src/lang/ast/query-items/field-declaration.ts index f58816926..6d3315f7e 100644 --- a/packages/malloy/src/lang/ast/query-items/field-declaration.ts +++ b/packages/malloy/src/lang/ast/query-items/field-declaration.ts @@ -36,6 +36,10 @@ import type {Noteable} from '../types/noteable'; import type {DynamicSpace} from '../field-space/dynamic-space'; import {SpaceField} from '../types/space-field'; import {DefSpace} from '../field-space/def-space'; +import { + booleanValueError, + resultColumnHint, +} from '../expressions/boolean-value'; export type FieldDeclarationConstructor = new ( expr: ExpressionDef, @@ -81,6 +85,11 @@ export abstract class AtomicFieldDeclaration return false; } + // The declaration is a result column of a query + outputsValue(): boolean { + return false; + } + queryFieldDef(exprFS: FieldSpace, exprName: string): AtomicFieldDef { let exprValue: ExprValue; @@ -121,28 +130,24 @@ export abstract class AtomicFieldDeclaration } if (isAtomicFieldType(exprValue.type) && exprValue.type !== 'error') { this.typecheckExprValue(exprValue); + if ( + this.outputsValue() && + booleanValueError( + exprFS, + exprValue, + 'as a result column', + this.expr, + resultColumnHint(this.exprSrc ?? '…') + ) + ) { + return {name: exprName, type: 'error'}; + } const ret = mkFieldDef(TDU.atomicDef(exprValue), exprName); if (TD.isTemporal(ret) && isGranularResult(exprValue)) { ret.timeframe = exprValue.timeframe; } ret.location = this.location; - if ( - exprValue.type === 'boolean' && - exprFS.dialectObj()?.booleanType === 'none' - ) { - // when generating a boolean field on a database without boolean support - // map it to integers - ret.e = { - node: 'case', - kids: { - caseWhen: [exprValue.value], - caseThen: [{node: 'numberLiteral', literal: '1'}], - caseElse: {node: 'numberLiteral', literal: '0'}, - }, - }; - } else { - ret.e = exprValue.value; - } + ret.e = exprValue.value; ret.drillExpression = this.expr.drillExpression(); ret.refSummary = exprValue.refSummary; ret.ungroupings = exprValue.ungroupings; @@ -181,6 +186,9 @@ export abstract class AtomicFieldDeclaration export class CalculateFieldDeclaration extends AtomicFieldDeclaration { elementType = 'calculateFieldDeclaration'; + outputsValue(): boolean { + return true; + } typecheckExprValue(expr: ExprValue) { typecheckCalculate(expr, this); } @@ -191,6 +199,9 @@ export class CalculateFieldDeclaration extends AtomicFieldDeclaration { export class AggregateFieldDeclaration extends AtomicFieldDeclaration { elementType = 'aggregateFieldDeclaration'; + outputsValue(): boolean { + return true; + } typecheckExprValue(expr: ExprValue) { typecheckAggregate(expr, this); } @@ -198,6 +209,9 @@ export class AggregateFieldDeclaration extends AtomicFieldDeclaration { export class GroupByFieldDeclaration extends AtomicFieldDeclaration { elementType = 'groupByFieldDeclaration'; + outputsValue(): boolean { + return true; + } typecheckExprValue(expr: ExprValue) { typecheckGroupBy(expr, this); } @@ -205,6 +219,9 @@ export class GroupByFieldDeclaration extends AtomicFieldDeclaration { export class ProjectFieldDeclaration extends AtomicFieldDeclaration { elementType = 'projectFieldDeclaration'; + outputsValue(): boolean { + return true; + } typecheckExprValue(expr: ExprValue) { typecheckProject(expr, this); } diff --git a/packages/malloy/src/lang/ast/query-items/field-references.ts b/packages/malloy/src/lang/ast/query-items/field-references.ts index 1a0ea4a54..be64a5402 100644 --- a/packages/malloy/src/lang/ast/query-items/field-references.ts +++ b/packages/malloy/src/lang/ast/query-items/field-references.ts @@ -20,6 +20,10 @@ import {ListOf, MalloyElement} from '../types/malloy-element'; import type {Noteable} from '../types/noteable'; import type {MakeEntry} from '../types/space-entry'; import {ExprIdReference} from '../expressions/expr-id-reference'; +import { + booleanValueError, + resultColumnHint, +} from '../expressions/boolean-value'; import type {FieldDeclarationConstructor} from './field-declaration'; import { AggregateFieldDeclaration, @@ -175,12 +179,26 @@ export abstract class FieldReference abstract typecheck(type: TypeDesc): void; + // The reference is a result column of a query + outputsValue(): boolean { + return false; + } + getField(fs: FieldSpace): LookupResult { const result = fs.lookup(this.list); if (result.found) { const actualType = result.found.typeDesc(); this.typecheck(actualType); + if (this.outputsValue()) { + booleanValueError( + fs, + actualType, + 'as a result column', + this, + resultColumnHint(this.refString) + ); + } } return result; @@ -252,6 +270,9 @@ export class DrillFieldReference extends FieldReference { export class CalculateFieldReference extends FieldReference { elementType = 'calculateFieldReference'; + outputsValue(): boolean { + return true; + } protected autoRenameDeclarationCtor() { return CalculateFieldDeclaration; } @@ -262,6 +283,9 @@ export class CalculateFieldReference extends FieldReference { export class IndexFieldReference extends FieldReference { elementType = 'indexFieldReference'; + outputsValue(): boolean { + return true; + } typecheck(type: TypeDesc) { typecheckIndex(type, this); } @@ -278,6 +302,9 @@ export class IndexFieldReference extends FieldReference { export class AggregateFieldReference extends FieldReference { elementType = 'aggregateFieldReference'; + outputsValue(): boolean { + return true; + } protected autoRenameDeclarationCtor() { return AggregateFieldDeclaration; } @@ -288,6 +315,9 @@ export class AggregateFieldReference extends FieldReference { export class GroupByFieldReference extends FieldReference { elementType = 'groupByFieldReference'; + outputsValue(): boolean { + return true; + } protected autoRenameDeclarationCtor() { return GroupByFieldDeclaration; } @@ -298,6 +328,9 @@ export class GroupByFieldReference extends FieldReference { export class ProjectFieldReference extends FieldReference { elementType = 'projectFieldReference'; + outputsValue(): boolean { + return true; + } protected autoRenameDeclarationCtor() { return ProjectFieldDeclaration; } diff --git a/packages/malloy/src/lang/ast/query-properties/filters.ts b/packages/malloy/src/lang/ast/query-properties/filters.ts index a05e5edc5..56f88c995 100644 --- a/packages/malloy/src/lang/ast/query-properties/filters.ts +++ b/packages/malloy/src/lang/ast/query-properties/filters.ts @@ -64,10 +64,12 @@ export class FilterElement extends MalloyElement { filterCondition(fs: FieldSpace): FilterCondition { const exprVal = this.expr.getExpression(fs); if (exprVal.type !== 'boolean') { - this.expr.logError( - 'non-boolean-filter', - 'Filter expression must have boolean value' - ); + if (exprVal.type !== 'error') { + this.expr.logError( + 'non-boolean-filter', + 'Filter expression must have boolean value' + ); + } return { node: 'filterCondition', code: this.exprSrc, diff --git a/packages/malloy/src/lang/ast/types/expression-def.ts b/packages/malloy/src/lang/ast/types/expression-def.ts index bdb63acd4..ca51a54ed 100644 --- a/packages/malloy/src/lang/ast/types/expression-def.ts +++ b/packages/malloy/src/lang/ast/types/expression-def.ts @@ -44,6 +44,7 @@ import { TemporalFilterExpression, isFilterable, } from '@malloydata/malloy-filter'; +import {booleanValueError, COMPARE_HINT} from '../expressions/boolean-value'; class TypeMismatch extends Error {} @@ -419,6 +420,23 @@ function regexEqual(left: ExprValue, right: ExprValue): Expr | undefined { return undefined; } +/** + * A boolean can only be compared, with anything, on a dialect with boolean + * values. Every comparison operator, applied plainly, as a partial, or + * through an alternation, arrives here through equality() or compare(). + */ +function booleanCompare( + fs: FieldSpace, + left: ExpressionDef, + lhs: ExprValue, + op: CompareMalloyOperator, + right: ExpressionDef, + rhs: ExprValue +): ExprValue | undefined { + const [side, val] = lhs.type === 'boolean' ? [left, lhs] : [right, rhs]; + return booleanValueError(fs, val, `compared with ${op}`, side, COMPARE_HINT); +} + function equality( fs: FieldSpace, left: ExpressionDef, @@ -429,7 +447,9 @@ function equality( const rhs = right.getExpression(fs); const node = getExprNode(op); - const err = errorCascade('boolean', lhs, rhs); + const err = + errorCascade('boolean', lhs, rhs) || + booleanCompare(fs, left, lhs, op, right, rhs); if (err) return err; // Unsupported types can be compare with null @@ -454,6 +474,14 @@ function equality( `Cannot use the '${op}' operator with a filter expression` ); } + const noMatch = booleanValueError( + fs, + lhs, + 'matched with a filter expression', + left, + 'test the condition itself' + ); + if (noMatch) return noMatch; if (isFilterable(lhs.type)) { let actualFilter = rhs.value; while (actualFilter.node === '()') { @@ -519,7 +547,9 @@ function compare( const lhs = left.getExpression(fs); const rhs = right.getExpression(fs); - const err = errorCascade('boolean', lhs, rhs); + const err = + errorCascade('boolean', lhs, rhs) || + booleanCompare(fs, left, lhs, op, right, rhs); if (err) return err; const noCompare = unsupportError(left, lhs, right, rhs); diff --git a/packages/malloy/src/lang/ast/view-elements/reference-view.ts b/packages/malloy/src/lang/ast/view-elements/reference-view.ts index b6869be24..8372b42fa 100644 --- a/packages/malloy/src/lang/ast/view-elements/reference-view.ts +++ b/packages/malloy/src/lang/ast/view-elements/reference-view.ts @@ -15,6 +15,10 @@ import { sourceBase, } from '../../../model/malloy_types'; import {ErrorFactory} from '../error-factory'; +import { + booleanValueError, + resultColumnHint, +} from '../expressions/boolean-value'; import type {QueryOperationSpace} from '../field-space/query-spaces'; import type {ViewOrScalarFieldReference} from '../query-items/field-references'; import {attachDrillPaths} from '../query-properties/drill'; @@ -74,6 +78,17 @@ export class ReferenceView extends View { throw new Error('Expected field to have definition'); } if (isAtomic(fieldDef)) { + if ( + booleanValueError( + fs, + fieldDef, + 'as a result column', + this.reference, + resultColumnHint(this.reference.refString) + ) + ) { + return oops(); + } const name = this.reference.nameString; const outputStruct: SourceDef = { ...sourceBase(fs.structDef()), diff --git a/packages/malloy/src/lang/parse-log.ts b/packages/malloy/src/lang/parse-log.ts index d58862c6b..e24fbc22e 100644 --- a/packages/malloy/src/lang/parse-log.ts +++ b/packages/malloy/src/lang/parse-log.ts @@ -94,6 +94,13 @@ type MessageParameterTypes = { 'experiment-not-enabled': {experimentId: string}; 'experimental-dialect-not-enabled': {dialect: string}; 'nesting-unsupported': {dialect: string}; + 'boolean-value-unsupported': {dialect: string; where: string; hint: string}; + 'wildcard-omits-boolean': { + dialect: string; + wildcard: string; + name: string; + hint: string; + }; 'nested-multi-stage-unsupported': {dialect: string}; 'nested-projection-limit-unsupported': {dialect: string}; 'sql-native-not-allowed-in-expression': { @@ -466,6 +473,10 @@ export const MESSAGE_FORMATTERS: PartialErrorCodeMessageMap = { 'experimental-dialect-not-enabled': e => `Requires compiler flag '##! experimental.dialect.${e.dialect}'`, 'nesting-unsupported': e => `'${e.dialect}' does not support nested queries`, + 'boolean-value-unsupported': e => + `'${e.dialect}' does not support a boolean ${e.where}; ${e.hint}`, + 'wildcard-omits-boolean': e => + `'${e.name}' is left out of '${e.wildcard}': '${e.dialect}' does not support a boolean as a result column; ${e.hint}`, 'nested-multi-stage-unsupported': e => `'${e.dialect}' does not support a multi-stage pipeline ('->') in a nested query`, 'nested-projection-limit-unsupported': e => diff --git a/packages/malloy/src/lang/test/CONTEXT.md b/packages/malloy/src/lang/test/CONTEXT.md index 75d9b08cb..8a3cb77b8 100644 --- a/packages/malloy/src/lang/test/CONTEXT.md +++ b/packages/malloy/src/lang/test/CONTEXT.md @@ -14,18 +14,20 @@ The tests use a mock schema system that provides pre-defined tables and sources, `TestTranslator` extends `MalloyTranslator` with a mock schema and pre-defined model. It provides: -- **Four connections:** +- **Five connections:** - `_db_` - DuckDB dialect, supports `timestamptz` - `_db2_` - DuckDB dialect, a second connection on the same dialect as `_db_`, so a test can write a cross-connection reference which is not also a cross-dialect one - `_bq_` - BigQuery/StandardSQL dialect - `_pg_` - Postgres dialect, the only dialect with `hasFinalStage` + - `_mssql_` - SQL Server dialect, the only dialect with `booleanType: 'none'` - **Mock tables** (accessible via `connection.table('path')`): - `_db_.table('aTable')` - base table with all field types - `_db2_.table('aTable')` - same table, on the second connection - `_bq_.table('aTable')` - BigQuery version - `_pg_.table('aTable')` - Postgres version + - `_mssql_.table('aTable')` - SQL Server version, without the boolean column - `_db_.table('malloytest.carriers')` - `_db_.table('malloytest.flights')` - `_db_.table('malloytest.airports')` diff --git a/packages/malloy/src/lang/test/boolean-coercion.spec.ts b/packages/malloy/src/lang/test/boolean-coercion.spec.ts new file mode 100644 index 000000000..53425c8f2 --- /dev/null +++ b/packages/malloy/src/lang/test/boolean-coercion.spec.ts @@ -0,0 +1,259 @@ +/* + * Copyright Contributors to the Malloy project + * SPDX-License-Identifier: MIT + */ + +import './parse-expects'; +import {TestTranslator} from './test-translator'; +import {QueryModel} from '../../model'; + +// In the boolean_coercion experiment, a boolean on a dialect with +// booleanType 'none' is still a condition, and the compiler turns it into +// a value (1, 0, or NULL) wherever SQL wants a value, and a value back into +// a condition wherever SQL wants a condition. Nothing is refused. +const prefix = `##! experimental { dialect.sqlserver parameters sql_functions boolean_coercion } +source: m is _mssql_.table('aTable') extend { + dimension: big is ai > 5 + dimension: sb is sql_boolean("""ai > 7""") + measure: n is count() + measure: anyn is n > 1 + join_one: o is _mssql_.table('aTable') extend { dimension: big is ai > 5 } on o.ai = ai +} +source: pk is _mssql_.table('aTable') extend { + dimension: big is ai > 5 + primary_key: big +} +source: pkj is _mssql_.table('aTable') extend { + dimension: big is ai > 5 + join_one: pk on pk.ai = ai +} +source: pm(p::boolean is true) is _mssql_.table('aTable') extend { + dimension: pv is p + measure: n is count() +} +`; + +function sqlOf(query: string): string { + const tt = new TestTranslator(prefix + query); + expect(tt).toTranslate(); + const modelDef = tt.translate().modelDef!; + const ql = modelDef.queryList; + const compiled = new QueryModel(modelDef).compileQuery(ql[ql.length - 1]); + return compiled.sql.replace(/\s+/g, ' '); +} + +const bigCond = '(base."ai">5)'; +// At the root of a field's expression the comparison is not wrapped +const bigValue = + 'CASE WHEN base."ai">5 THEN 1 WHEN NOT (base."ai">5) THEN 0 END'; +// Through a reference to the field, it is +const bigRef = `(${bigValue})`; +const trueValue = 'CASE WHEN (1=1) THEN 1 WHEN NOT ((1=1)) THEN 0 END'; + +describe('boolean coercion on a dialect without boolean values', () => { + describe('a condition where a value is wanted becomes 1, 0 or NULL', () => { + test('as a result column', () => { + expect(sqlOf('run: m -> { group_by: big }')).toContain( + `${bigValue} as "big"` + ); + }); + test('as a selected column', () => { + expect(sqlOf('run: m -> { select: big }')).toContain( + `${bigValue} as "big"` + ); + }); + test('in select: *, with no warning', () => { + expect(sqlOf('run: m -> { select: * }')).toContain( + `${bigValue} as "big"` + ); + }); + test('a literal', () => { + expect(sqlOf('run: m -> { group_by: t is true }')).toContain( + `${trueValue} as "t"` + ); + }); + test('a boolean function', () => { + const sql = sqlOf("run: m -> { group_by: s is starts_with(astr, 'a') }"); + expect(sql).toMatch( + /CASE WHEN LEFT\(.*\) = N'a' THEN 1 WHEN NOT \(LEFT\(.*\) = N'a'\) THEN 0 END as "s"/ + ); + }); + test('a sql_boolean', () => { + expect(sqlOf('run: m -> { group_by: sb }')).toContain( + 'CASE WHEN ai > 7 THEN 1 WHEN NOT (ai > 7) THEN 0 END as "sb"' + ); + }); + test('a boolean parameter', () => { + expect(sqlOf('run: pm -> { group_by: pv }')).toContain( + `${trueValue} as "pv"` + ); + }); + test('compared', () => { + expect(sqlOf('run: m -> { where: big = true; aggregate: n }')).toContain( + `WHERE ${bigRef}=${trueValue}` + ); + }); + test('null tested', () => { + expect(sqlOf('run: m -> { where: big is null; aggregate: n }')).toContain( + `WHERE ${bigRef} IS NULL` + ); + }); + test('as a function argument', () => { + expect(sqlOf('run: m -> { group_by: s is concat(big) }')).toContain( + `CONCAT(${bigRef})` + ); + }); + test('as a pick result', () => { + expect( + sqlOf('run: m -> { group_by: x is pick big when ai > 1 else false }') + ).toContain( + `CASE WHEN (base."ai">1) THEN ${bigRef} ELSE CASE WHEN (1=0) THEN 1 WHEN NOT ((1=0)) THEN 0 END END as "x"` + ); + }); + test('cast', () => { + expect(sqlOf('run: m -> { group_by: x is big::number }')).toContain( + `CAST(${bigRef} AS` + ); + }); + test('in a window function', () => { + expect( + sqlOf('run: m -> { group_by: big; calculate: x is lag(big) }') + ).toContain(`LAG(${bigRef})`); + }); + test('as a primary key, hashed and counted', () => { + expect(sqlOf('run: pkj -> { aggregate: x is pk.count() }')).toContain( + 'COUNT(DISTINCT (CASE WHEN pk_0."ai">5 THEN 1 WHEN NOT (pk_0."ai">5) THEN 0 END))' + ); + expect(sqlOf('run: pkj -> { aggregate: x is pk.ai.sum() }')).toContain( + 'HASHBYTES(\'MD5\', CAST((CASE WHEN pk_0."ai">5 THEN 1 WHEN NOT (pk_0."ai">5) THEN 0 END) AS NVARCHAR(MAX)))' + ); + }); + }); + + describe('a value where a condition is wanted is tested against 1', () => { + test('an ungrouped boolean measure in having:', () => { + expect( + sqlOf('run: m -> { group_by: ai; aggregate: n; having: all(anyn) }') + ).toContain( + 'HAVING (MAX((CASE WHEN (COUNT(1))>1 THEN 1 WHEN NOT ((COUNT(1))>1) THEN 0 END)) OVER () = 1)' + ); + }); + test('a boolean column from an earlier stage', () => { + expect( + sqlOf('run: m -> { group_by: ai, big } -> { where: big; select: ai }') + ).toContain('WHERE (base."big" = 1)'); + }); + test('a pick of conditions', () => { + expect( + sqlOf( + 'run: m -> { where: pick big when ai > 1 else false; aggregate: n }' + ) + ).toContain( + `WHERE (CASE WHEN (base."ai">1) THEN ${bigRef} ELSE CASE WHEN (1=0) THEN 1 WHEN NOT ((1=0)) THEN 0 END END = 1)` + ); + }); + test('?? of booleans', () => { + expect( + sqlOf('run: m -> { where: big ?? false; aggregate: n }') + ).toContain( + `WHERE (COALESCE(${bigRef},CASE WHEN (1=0) THEN 1 WHEN NOT ((1=0)) THEN 0 END) = 1)` + ); + }); + }); + + describe('a condition where a condition is wanted is unchanged', () => { + test('where:', () => { + const sql = sqlOf('run: m -> { where: big; aggregate: n }'); + expect(sql).toContain(`WHERE ${bigCond}`); + expect(sql).not.toContain('CASE'); + }); + test('having:', () => { + const sql = sqlOf('run: m -> { aggregate: n; having: anyn }'); + expect(sql).toContain('HAVING ((COUNT(1))>1)'); + expect(sql).not.toContain('CASE'); + }); + test('a join on:', () => { + const sql = sqlOf( + 'source: j is m extend { join_one: q is m on q.ai = ai and big }\nrun: j -> { aggregate: x is q.n }' + ); + expect(sql).toContain('ON (q_0."ai"=base."ai") and (base."ai">5)'); + expect(sql).not.toContain('CASE WHEN (base."ai">5) THEN 1'); + }); + test('a source where:', () => { + const sql = sqlOf( + 'source: s2 is m extend { where: big }\nrun: s2 -> { aggregate: n }' + ); + expect(sql).toContain(`WHERE ${bigCond}`); + expect(sql).not.toContain('CASE'); + }); + test('a filtered aggregate', () => { + const sql = sqlOf('run: m -> { aggregate: x is n { where: big } }'); + expect(sql).toContain(`COUNT(CASE WHEN ${bigCond} THEN 1 END)`); + }); + test('pick … when', () => { + const sql = sqlOf( + "run: m -> { group_by: x is pick 'yes' when big else 'no' }" + ); + expect(sql).toContain(`CASE WHEN ${bigCond} THEN N'yes' ELSE N'no' END`); + }); + test('not, and, or', () => { + const sql = sqlOf( + 'run: m -> { where: not big and (big or ai = 1); aggregate: n }' + ); + expect(sql).toContain( + `WHERE (CASE WHEN ${bigCond} THEN 1 ELSE 0 END = 0) and ((${bigCond} or (base."ai"=1)))` + ); + }); + test('a boolean function as a filter', () => { + const sql = sqlOf( + "run: m -> { where: starts_with(astr, 'a'); aggregate: n }" + ); + expect(sql).toMatch(/WHERE LEFT\(/); + expect(sql).not.toContain('CASE'); + }); + test('raw SQL decides for itself', () => { + const sql = sqlOf( + 'run: m -> { group_by: x is sql_number("""CASE WHEN ${big} THEN 1 ELSE 0 END""") }' + ); + expect(sql).toContain(`CASE WHEN ${bigCond} THEN 1 ELSE 0 END as "x"`); + }); + test('a boolean filter expression', () => { + expect( + sqlOf("run: m -> { where: big ~ f'true'; aggregate: n }") + ).toContain(`WHERE ${bigCond}`); + expect( + sqlOf("run: m -> { where: big ~ f'not true'; aggregate: n }") + ).toContain(`WHERE (CASE WHEN ${bigCond} THEN 1 ELSE 0 END = 0)`); + expect( + sqlOf("run: m -> { where: big ~ f'=false'; aggregate: n }") + ).toContain(`WHERE NOT (${bigCond})`); + expect( + sqlOf("run: m -> { where: big ~ f'null'; aggregate: n }") + ).toContain( + `WHERE CASE WHEN ${bigCond} THEN 1 WHEN NOT (${bigCond}) THEN 0 END IS NULL` + ); + }); + test('a boolean parameter as a filter', () => { + const sql = sqlOf('run: pm -> { where: pv; aggregate: n }'); + expect(sql).toContain('WHERE ((1=1))'); + }); + }); + + test('a cast to boolean is still refused: there is no value to make', () => { + const tt = new TestTranslator( + prefix + 'run: m -> { where: ai::boolean; aggregate: n }' + ); + const problems = tt.translate().problems ?? []; + expect(problems.map(p => p.code)).toContain('boolean-value-unsupported'); + }); + + test('without the experiment a boolean value is still refused', () => { + const tt = new TestTranslator( + `##! experimental.dialect.sqlserver +source: m is _mssql_.table('aTable') extend { dimension: big is ai > 5 } +run: m -> { group_by: big }` + ); + const problems = tt.translate().problems ?? []; + expect(problems.map(p => p.code)).toContain('boolean-value-unsupported'); + }); +}); diff --git a/packages/malloy/src/lang/test/boolean-none.spec.ts b/packages/malloy/src/lang/test/boolean-none.spec.ts new file mode 100644 index 000000000..01aa9a62a --- /dev/null +++ b/packages/malloy/src/lang/test/boolean-none.spec.ts @@ -0,0 +1,222 @@ +/* + * Copyright Contributors to the Malloy project + * SPDX-License-Identifier: MIT + */ + +import './parse-expects'; +import { + error, + errorMessage, + makeModelFunc, + model, + warningMessage, +} from './test-translator'; + +// On a dialect with booleanType 'none' a boolean is a condition and never a +// value. `m` is a SQL Server source with a declared boolean, `big`, a boolean +// measure, `anyn`, and a view, `v`. In a refusal test, the `${…}` marks the +// expression the error is expected at. +const m = makeModelFunc({ + prefix: `##! experimental.dialect.sqlserver +source: m is _mssql_.table('aTable') extend { + dimension: big is ai > 5 + measure: n is count() + measure: anyn is n > 1 + view: v is { group_by: ai } + join_one: o is _mssql_.table('aTable') extend { dimension: big is ai > 5 } on o.ai = ai +} +`, +}); + +// A refusal is checked by its code, and by the `${…}` mark saying where it +// landed; the hint's wording is checked only where the hint is the point. +const refused = error('boolean-value-unsupported'); + +describe('a boolean on a dialect without boolean values', () => { + describe('is a condition', () => { + test('in where: and having:', () => { + expect( + m`run: m -> { where: big; aggregate: n; having: n > 1 }` + ).toTranslate(); + }); + test('in pick … when', () => { + expect( + m`run: m -> { group_by: size is pick 'big' when big else 'small' }` + ).toTranslate(); + }); + test('in a filtered aggregate', () => { + expect( + m`run: m -> { aggregate: bigs is n { where: big } }` + ).toTranslate(); + }); + test('in a join on:', () => { + expect( + m`source: j is m extend { join_one: p is m on big and p.ai = ai }` + ).toTranslate(); + }); + test('under not, and, or', () => { + expect( + m`run: m -> { where: not big and (big or ai = 1); aggregate: n }` + ).toTranslate(); + }); + test('as a declared dimension or measure', () => { + expect( + m`source: m2 is m extend { dimension: small is not big; measure: none is n = 0 }` + ).toTranslate(); + }); + }); + + describe('is refused as a value', () => { + test('a declared boolean in group_by:', () => { + expect(m`run: m -> { group_by: ${'big'}; aggregate: n }`).toLog( + errorMessage( + "'sqlserver' does not support a boolean as a result column; write the value, pick 'yes' when big else 'no'" + ) + ); + }); + test('a comparison in select:', () => { + expect(m`run: m -> { select: is_big is ${'ai > 5'} }`).toLog( + errorMessage( + "'sqlserver' does not support a boolean as a result column; write the value, pick 'yes' when ai > 5 else 'no'" + ) + ); + }); + test('a boolean aggregate', () => { + expect(m`run: m -> { aggregate: any_big is ${'n > 1'} }`).toLog(refused); + }); + test('in index:', () => { + expect(m`run: m -> { index: ${'big'} }`).toLog(refused); + }); + test('a boolean field named as the whole view', () => { + expect(m`run: m -> ${'big'}`).toLog(refused); + expect(m`run: m -> ${'anyn'}`).toLog(refused); + expect(m`run: m -> v + ${'big'}`).toLog(refused); + }); + test('in a null test', () => { + expect(m`run: m -> { where: ${'big'} is null; aggregate: n }`).toLog( + refused + ); + }); + test('compared with = or !=', () => { + expect(m`run: m -> { where: ${'big'} = true; aggregate: n }`).toLog( + refused + ); + expect(m`run: m -> { where: ${'big'} != (ai > 6); aggregate: n }`).toLog( + refused + ); + }); + test('compared with something which is not a boolean', () => { + expect(m`run: m -> { where: ${'big'} = 1; aggregate: n }`).toLog(refused); + expect(m`run: m -> { where: ai = ${'big'}; aggregate: n }`).toLog( + refused + ); + expect(m`run: m -> { where: null = ${'big'}; aggregate: n }`).toLog( + refused + ); + expect(m`run: m -> { where: ${'big'} ? > 1; aggregate: n }`).toLog( + refused + ); + expect(m`run: m -> { where: astr != ${'big'}; aggregate: n }`).toLog( + refused + ); + }); + test('compared with an ordering operator', () => { + expect(m`run: m -> { where: ${'big'} > false; aggregate: n }`).toLog( + refused + ); + expect(m`run: m -> { where: ${'big'} ? > false; aggregate: n }`).toLog( + refused + ); + expect( + m`run: m -> { where: ${'big'} ? > false | < true; aggregate: n }` + ).toLog(refused, refused); + }); + test('in an alternation', () => { + expect( + m`run: m -> { where: ${'big'} ? true | false; aggregate: n }` + ).toLog(refused); + expect(m`run: m -> { where: ai ? 1 | ${'big'}; aggregate: n }`).toLog( + refused + ); + }); + test('in ??', () => { + expect(m`run: m -> { where: ${'big ?? false'}; aggregate: n }`).toLog( + refused + ); + }); + test('as the result of a pick', () => { + expect( + m`run: m -> { where: ${'pick true when ai > 1 else false'}; aggregate: n }` + ).toLog(refused); + }); + test('as the result of a case', () => { + expect( + m`run: m -> { where: case when ai > 1 then true else false end; aggregate: n }` + ).toLog( + warningMessage('Use a `pick` statement instead of `case`'), + refused + ); + }); + test('matched with a filter expression', () => { + expect(m`run: m -> { where: ${'big'} ~ f'true'; aggregate: n }`).toLog( + refused + ); + }); + test('as a function argument', () => { + expect(m`run: m -> { group_by: s is concat('x', ${'big'}) }`).toLog( + refused + ); + expect(m`run: m -> { group_by: s is foo!number(${'big'}) }`).toLog( + refused + ); + }); + test('in a cast, either way', () => { + expect(m`run: m -> { group_by: s is ${'big::string'} }`).toLog(refused); + expect(m`run: m -> { where: ${'ai::boolean'}; aggregate: n }`).toLog( + refused + ); + }); + test('once, at the expression which used it', () => { + expect( + m`run: m -> { group_by: x is ${'pick big when ai > 1 else big'} }` + ).toLog(refused); + }); + test('once, through a unary minus', () => { + expect(m`run: m -> { where: -(${'big ?? false'}); aggregate: n }`).toLog( + refused + ); + }); + }); + + describe('select: * leaves a boolean out, with a warning', () => { + test('naming the field, and not a measure', () => { + expect(m`run: m -> { select: * }`).toLog( + warningMessage( + "'big' is left out of '*': 'sqlserver' does not support a boolean as a result column; write the value, pick 'yes' when big else 'no'" + ) + ); + }); + test('naming a joined field by its path', () => { + expect(m`run: m -> { select: o.* }`).toLog( + warningMessage( + "'o.big' is left out of 'o.*': 'sqlserver' does not support a boolean as a result column; write the value, pick 'yes' when o.big else 'no'" + ) + ); + }); + test('and so does index: *', () => { + expect(m`run: m -> { index: * }`).toLog( + warningMessage( + "'big' is left out of '*': 'sqlserver' does not support a boolean as a result column; write the value, pick 'yes' when big else 'no'" + ) + ); + }); + }); + + test('the same boolean is a value on a dialect with booleans', () => { + expect(model` + source: d is _db_.table('aTable') extend { dimension: big is ai > 5 } + run: d -> { group_by: big, s is big::string, c is concat('x', big); where: big = true and big > false } + run: d -> big + `).toTranslate(); + }); +}); diff --git a/packages/malloy/src/lang/test/expressions.spec.ts b/packages/malloy/src/lang/test/expressions.spec.ts index b74dabb51..5f3da3227 100644 --- a/packages/malloy/src/lang/test/expressions.spec.ts +++ b/packages/malloy/src/lang/test/expressions.spec.ts @@ -126,6 +126,16 @@ describe('expressions', () => { test('unary negation', () => { expect('- ai').compilesTo('{unary- ai}'); }); + test('unary negation of a non-number is an error', () => { + expect(expr`- ${'astr'}`).toLog( + errorMessage("'unary minus' Can't use type string") + ); + }); + test('unary negation of an error is that error, logged once', () => { + expect( + `run: a -> { where: -(-${'astr'}); aggregate: c is count() }` + ).toLog(errorMessage("'unary minus' Can't use type string")); + }); test('equal', () => { expect('42 = 7').compilesTo('{42 = 7}'); }); diff --git a/packages/malloy/src/lang/test/parse.spec.ts b/packages/malloy/src/lang/test/parse.spec.ts index f417c8572..56edc4917 100644 --- a/packages/malloy/src/lang/test/parse.spec.ts +++ b/packages/malloy/src/lang/test/parse.spec.ts @@ -331,7 +331,7 @@ describe('error cascading', () => { 'err.month': 'date', 'day_of_week(err)': 'number', 'err::string': 'string', - '-err': 'number', + '-err': 'error', 'err * 1': 'number', '1 * err': 'number', 'err / 1': 'number', diff --git a/packages/malloy/src/lang/test/test-translator.ts b/packages/malloy/src/lang/test/test-translator.ts index 2b23c7658..858a9856d 100644 --- a/packages/malloy/src/lang/test/test-translator.ts +++ b/packages/malloy/src/lang/test/test-translator.ts @@ -223,6 +223,17 @@ export const pgTableDef: SourceDef = { fields: baseFields, }; +// SQL Server has no boolean values (booleanType 'none'), so no column of +// its table is a boolean +export const mssqlTableDef: SourceDef = { + type: 'table', + name: 'aTable', + dialect: 'sqlserver', + tablePath: 'aTable', + connection: '_mssql_', + fields: baseFields.filter(f => f.name !== 'abool'), +}; + /** * A TestTranlator never actually talks to connection, instead uses * some mocked schema definitions. @@ -233,6 +244,7 @@ export const mockSchema: TableSourceDef[] = [ db2TableDef, bqTableDef, pgTableDef, + mssqlTableDef, { type: 'table', name: 'carriers', @@ -396,7 +408,7 @@ export interface TestTranslatorOptions { /** * Every connection name a test mentions resolves to a dialect, as it would - * through a host's connection lookup: the four listed below to theirs, and + * through a host's connection lookup: the five listed below to theirs, and * any other name to `TEST_DIALECT`. */ class TestConnectionDialectZone extends Zone { @@ -412,7 +424,7 @@ export class TestTranslator extends MalloyTranslator { testRoot?: TestRoot; connectionDialectZone = new TestConnectionDialectZone(); /* - * There are four connections: + * There are five connections: * _db_ - duckdb dialect, with the following tables ... * aTable, malloytest.carriers, malloytest.flights, malloytest.airports * _db2_ - duckdb dialect, with one table, aTable. A second connection @@ -421,6 +433,8 @@ export class TestTranslator extends MalloyTranslator { * aTable * _pg_ - postgres dialect, with one table * aTable + * _mssql_ - sqlserver dialect, which has no boolean values, with one + * table, aTable * * The "aTable" table is a mocked table with one column of each type. * The _bq_ version does not have the timestamptz column, and when @@ -450,6 +464,7 @@ export class TestTranslator extends MalloyTranslator { _db2_: {type: 'connection', name: '_db2_'}, _bq_: {type: 'connection', name: '_bq_'}, _pg_: {type: 'connection', name: '_pg_'}, + _mssql_: {type: 'connection', name: '_mssql_'}, a: {...aTableDef, primaryKey: 'astr', name: 'a'}, b: {...aTableDef, primaryKey: 'astr', name: 'b'}, bq_a: {...bqTableDef, primaryKey: 'astr', name: 'bq_a'}, @@ -523,6 +538,7 @@ export class TestTranslator extends MalloyTranslator { this.connectionDialectZone.define('_db2_', TEST_DIALECT); this.connectionDialectZone.define('_bq_', 'standardsql'); this.connectionDialectZone.define('_pg_', 'postgres'); + this.connectionDialectZone.define('_mssql_', 'sqlserver'); for (const flag of options.compilerFlags ?? []) { this.compilerFlagSrc.push(`##! ${flag}\n`); } @@ -780,6 +796,10 @@ export function makeModelFunc( const {prefix, wrap, ...ttOptions} = options; return { ...ms, + // A marked location is where the mark sits in the whole source, after + // the prefix. A wrap could put the code anywhere, so marks stay as + // they are. + locations: wrap ? ms.locations : ms.locations.map(l => after(prefix, l)), translator: new TestTranslator( (prefix ?? '') + (wrap ? wrap(ms.code) : ms.code), ttOptions @@ -788,6 +808,21 @@ export function makeModelFunc( }; } +function after(prefix: string | undefined, l: DocumentLocation) { + if (!prefix) return l; + const lines = prefix.split('\n'); + const dLine = lines.length - 1; + const dChar = lines[dLine].length; + const shift = (p: {line: number; character: number}) => ({ + line: p.line + dLine, + character: p.line === 0 ? p.character + dChar : p.character, + }); + return { + ...l, + range: {start: shift(l.range.start), end: shift(l.range.end)}, + }; +} + export function makeExprFunc( model?: ModelDef, sourceName: string = 'ab', diff --git a/packages/malloy/src/model/CONTEXT.md b/packages/malloy/src/model/CONTEXT.md index 12e404249..c0c83ed22 100644 --- a/packages/malloy/src/model/CONTEXT.md +++ b/packages/malloy/src/model/CONTEXT.md @@ -447,6 +447,15 @@ Compiles IR expression trees into SQL expressions. - Function calls - Field references and path expressions +**A boolean on a dialect without boolean values.** On a dialect with `booleanType: 'none'` +(SQL Server) a boolean is a condition and never a value: a declared boolean compiles to its +comparison, and nothing converts a condition into a `1`/`0`. Malloy's NULL rules for `not`, +`!=` and `!~` (true where SQL says NULL) go through `Dialect.sqlNot` / `sqlNullIsTrue`, which +keep the `COALESCE` spelling on every dialect with booleans and write the test as a condition +on one without. The translator refuses a boolean anywhere SQL would want a value, so the +compiler does not look for one; the design and the list of refused uses are in +[`doc/boolean-none.md`](../doc/boolean-none.md). + ### 3. Dialect System The compiler supports multiple SQL dialects through a plugin architecture. diff --git a/packages/malloy/src/model/expression_compiler.ts b/packages/malloy/src/model/expression_compiler.ts index a4afc760b..e318b6c41 100644 --- a/packages/malloy/src/model/expression_compiler.ts +++ b/packages/malloy/src/model/expression_compiler.ts @@ -116,12 +116,112 @@ function deepCopyExpr(expr: T): T { * Compiles an expression tree by mutating it in-place to set all .sql fields. * Assumes the expression tree is already a copy that can be mutated. */ +/** + * The kind of SQL an expression is: a value, or a condition. A dialect with + * boolean values has one kind; on a dialect without them a condition can + * only stand where SQL wants a condition, and a value only where it wants + * a value, so the compiler tracks which kind each edge wants and which kind + * each node produced, and asks the dialect to coerce where they differ. + */ +export type SQLKind = 'value' | 'condition' | 'raw'; + +/** + * The kind a node wants from the child named `kid`, given the kind the + * node itself was asked for. + */ +function wantFromKid(expr: Expr, kid: string, want: SQLKind): SQLKind { + switch (expr.node) { + case 'and': + case 'or': + case 'not': + case 'filterCondition': + return 'condition'; + case '()': + return want; + case 'case': + // A CASE is a value, so its branches are values; a when is a + // condition unless there is a subject to compare it with + if (kid === 'caseWhen') { + return expr.kids.caseValue ? 'value' : 'condition'; + } + return 'value'; + case 'filteredExpr': + return kid === 'filterList' ? 'condition' : 'value'; + case 'genericSQLExpr': + // SQL the author wrote says what it wants itself; a function's + // implementation wants values + return expr.authored ? 'raw' : 'value'; + case 'filterMatch': + return expr.dataType === 'boolean' ? 'condition' : 'value'; + default: + return 'value'; + } +} + +/** + * The kind a node produced. A node which passes its want through to its + * expression (a field, a parameter, a case) produced what it was asked for. + */ +function kindProduced(expr: Expr, want: SQLKind): SQLKind { + switch (expr.node) { + case '>': + case '<': + case '>=': + case '<=': + case '=': + case '!=': + case 'and': + case 'or': + case 'not': + case 'like': + case '!like': + case 'in': + case 'inGiven': + case 'regexpMatch': + case 'filterMatch': + case 'is-null': + case 'is-not-null': + case 'true': + case 'false': + case 'filterCondition': + return 'condition'; + case 'function_call': + return expr.overload.returnType.type === 'boolean' + ? 'condition' + : 'value'; + case 'genericSQLExpr': + return expr.typeDef?.type === 'boolean' ? 'condition' : 'value'; + case 'field': + case 'parameter': + case 'given': + case '()': + return want; + default: + return 'value'; + } +} + +function asKind( + dialect: Dialect, + sql: string, + produced: SQLKind, + want: SQLKind +): string { + if (produced === want || want === 'raw') { + return sql; + } + return produced === 'condition' + ? dialect.sqlConditionAsValue(sql) + : dialect.sqlValueAsCondition(sql); +} + function compileExpr( resultSet: FieldInstanceResult, context: QueryStruct, expr: T, state: GenerateState = new GenerateState(), - wrap = true + wrap = true, + want: SQLKind = 'value' ): T { /* * Translate the children first, and stash the translation @@ -129,38 +229,51 @@ function compileExpr( * it will have access to the translated children. */ if (exprHasE(expr)) { - compileExpr(resultSet, context, expr.e, state); + compileExpr( + resultSet, + context, + expr.e, + state, + true, + wantFromKid(expr, 'e', want) + ); } else if (exprHasKids(expr)) { - for (const kidExpr of Object.values(expr.kids)) { + for (const [kidName, kidExpr] of Object.entries(expr.kids)) { if (kidExpr === null) continue; + const kidWant = wantFromKid(expr, kidName, want); if (Array.isArray(kidExpr)) { for (const e of kidExpr) { - compileExpr(resultSet, context, e, state); + compileExpr(resultSet, context, e, state, true, kidWant); } } else { - compileExpr(resultSet, context, kidExpr, state); + compileExpr(resultSet, context, kidExpr, state, true, kidWant); } } } + const finish = (sql: string): T => { + const wrapped = wrap && exprHasKids(expr) ? `(${sql})` : sql; + expr.sql = asKind(context.dialect, wrapped, kindProduced(expr, want), want); + return expr; + }; + /* * Give the dialect a chance to translate this node */ const qi = resultSet.getQueryInfo(); const dialectSQL = context.dialect.exprToSQL(qi, expr); if (dialectSQL) { - expr.sql = wrap && exprHasKids(expr) ? `(${dialectSQL})` : dialectSQL; - return expr; + return finish(dialectSQL); } const sql = (() => { switch (expr.node) { case 'field': - return generateFieldFragment(resultSet, context, expr, state); + return generateFieldFragment(resultSet, context, expr, state, want); case 'parameter': - return generateParameterFragment(resultSet, context, expr, state); + return generateParameterFragment(resultSet, context, expr, state, want); case 'given': - return generateGivenFragment(resultSet, context, expr, state); + return generateGivenFragment(resultSet, context, expr, state, want); case 'filteredExpr': return generateFilterFragment(resultSet, context, expr, state); case 'all': @@ -226,7 +339,7 @@ function compileExpr( // Malloy inequality comparisons always return a boolean case '!=': { const notEqual = `${expr.kids.left.sql}!=${expr.kids.right.sql}`; - return `COALESCE(${notEqual},true)`; + return context.dialect.sqlNullIsTrue(notEqual); } case 'and': case 'or': @@ -242,7 +355,7 @@ function compileExpr( // null binding collapses to empty-set semantics — not the SQL // `IN (NULL)` shape, which has confusing NULL-membership rules. if (bound.node === 'null') { - return expr.not ? 'TRUE' : 'FALSE'; + return context.dialect.sqlBoolean(expr.not); } if (bound.node !== 'arrayLiteral') { throw new Error( @@ -250,7 +363,7 @@ function compileExpr( ); } if (bound.kids.values.length === 0) { - return expr.not ? 'TRUE' : 'FALSE'; + return context.dialect.sqlBoolean(expr.not); } const elemSqls = bound.kids.values.map(v => exprToSQL(resultSet, context, v, state) @@ -269,13 +382,14 @@ function compileExpr( expr.kids.right.literal ) : `${expr.kids.left.sql} ${likeIt} ${expr.kids.right.sql}`; - return expr.node === 'like' ? compare : `COALESCE(${compare},true)`; + return expr.node === 'like' + ? compare + : context.dialect.sqlNullIsTrue(compare); } case '()': return `(${expr.e.sql})`; case 'not': - // Malloy not operator always returns a boolean - return `COALESCE(NOT ${expr.e.sql},TRUE)`; + return context.dialect.sqlNot(expr.e.sql ?? ''); case 'unary-': return `-${expr.e.sql}`; case 'is-null': @@ -318,8 +432,7 @@ function compileExpr( } })(); - expr.sql = wrap && exprHasKids(expr) ? `(${sql})` : sql; - return expr; + return finish(sql); } /** @@ -330,13 +443,21 @@ export function exprToSQL( resultSet: FieldInstanceResult, context: QueryStruct, exprToTranslate: Expr, - state: GenerateState = new GenerateState() + state: GenerateState = new GenerateState(), + want: SQLKind = 'value' ): string { // Make a deep copy that we can mutate during compilation const exprCopy = deepCopyExpr(exprToTranslate); // Compile the copy, setting .sql on all nodes - const compiled = compileExpr(resultSet, context, exprCopy, state, false); + const compiled = compileExpr( + resultSet, + context, + exprCopy, + state, + false, + want + ); return compiled.sql!; } @@ -798,7 +919,8 @@ export function generateFieldFragment( resultSet: FieldInstanceResult, context: QueryStruct, expr: FieldnameNode, - state: GenerateState + state: GenerateState, + want: SQLKind = 'value' ): string { // find the structDef and return the path to the field... const fieldRef = context.getFieldByName(expr.path, expr.at); @@ -807,7 +929,8 @@ export function generateFieldFragment( resultSet, fieldRef.parent, fieldRef.fieldDef.e, - state + state, + want ); return `(${ret})`; } else { @@ -823,7 +946,7 @@ export function generateFieldFragment( } // The normal case - just generate the SQL reference - return sqlFullChildReference( + const ref = sqlFullChildReference( fieldRef.parent, fieldRef.fieldDef.name, fieldRef.parent.structDef.type === 'record' @@ -833,6 +956,11 @@ export function generateFieldFragment( } : undefined ); + // A boolean column holds the value a condition became + if (fieldRef.fieldDef.type === 'boolean' && want === 'condition') { + return context.dialect.sqlValueAsCondition(ref); + } + return ref; } } @@ -849,13 +977,14 @@ export function generateParameterFragment( resultSet: FieldInstanceResult, context: QueryStruct, expr: ParameterNode, - state: GenerateState + state: GenerateState, + want: SQLKind = 'value' ): string { const name = expr.path[0]; context.eventStream?.emit('source-argument-compiled', {name}); const argument = safeRecordGet(context.arguments(), name); if (argument?.value) { - return exprToSQL(resultSet, context, argument.value, state); + return exprToSQL(resultSet, context, argument.value, state, want); } throw new MalloyCompileError( `Parameter '${expr.path.join('.')}' has no value supplied.`, @@ -891,12 +1020,13 @@ export function generateGivenFragment( resultSet: FieldInstanceResult, context: QueryStruct, expr: GivenRefNode, - state: GenerateState + state: GenerateState, + want: SQLKind = 'value' ): string { // The bound expr may itself be a `$OTHER`-bearing expression; recursive // compile handles default chains. const bound = resolveGivenBoundExpr(context, expr); - return exprToSQL(resultSet, context, bound, state); + return exprToSQL(resultSet, context, bound, state, want); } function unsatisfiedGivenMessage(refName: string): string { @@ -916,7 +1046,9 @@ export function generateFilterFragment( ): string { const allWhere = new AndChain(state.whereSQL); for (const cond of expr.kids.filterList) { - allWhere.add(exprToSQL(resultSet, context, cond.e, state.withWhere())); + allWhere.add( + exprToSQL(resultSet, context, cond.e, state.withWhere(), 'condition') + ); } return exprToSQL( resultSet, @@ -1200,7 +1332,13 @@ export function* stringsFromSQLExpression( yield str; const expr = subExprList.shift(); if (expr) { - yield exprToSQL(resultSet, context, expr, state); + yield exprToSQL( + resultSet, + context, + expr, + state, + e.authored ? 'raw' : 'value' + ); } } } diff --git a/packages/malloy/src/model/field_instance.ts b/packages/malloy/src/model/field_instance.ts index a44ae9256..d82df73d9 100644 --- a/packages/malloy/src/model/field_instance.ts +++ b/packages/malloy/src/model/field_instance.ts @@ -64,7 +64,8 @@ export class FieldInstanceField implements FieldInstance { resultSet: FieldInstanceResult, context: QueryStruct, expr: Expr, - state?: GenerateState + state?: GenerateState, + want?: 'value' | 'condition' ) => string; static registerExpressionCompiler( @@ -72,7 +73,8 @@ export class FieldInstanceField implements FieldInstance { resultSet: FieldInstanceResult, context: QueryStruct, expr: Expr, - state?: GenerateState + state?: GenerateState, + want?: 'value' | 'condition' ) => string ) { FieldInstanceField.exprCompiler = compiler; @@ -110,7 +112,9 @@ export class FieldInstanceField implements FieldInstance { return FieldInstanceField.exprCompiler( this.parent, this.f.parent, - this.f.fieldDef.e + this.f.fieldDef.e, + undefined, + this.fieldUsage.type === 'where' ? 'condition' : 'value' ); } diff --git a/packages/malloy/src/model/filter_compilers.ts b/packages/malloy/src/model/filter_compilers.ts index 41a57d488..1f9c84ba4 100644 --- a/packages/malloy/src/model/filter_compilers.ts +++ b/packages/malloy/src/model/filter_compilers.ts @@ -64,8 +64,8 @@ function unlike(disLiked: string[], x: string) { // The `none` filter ("always false") compiles to a constant, independent of // column type or value; `not none` is "always true". Shared by the top-level // guard and every per-type compiler so the constant lives in one place. -function noneToSQL(not: boolean | undefined): string { - return not ? 'true' : 'false'; +function noneToSQL(not: boolean | undefined, d: Dialect): string { + return d.sqlBoolean(not === true); } export const FilterCompilers = { @@ -77,7 +77,7 @@ export const FilterCompilers = { qi: QueryInfo = {} ) { if (c === null) { - return 'true'; + return d.sqlBoolean(true); } // `none` is the always-false filter (companion to the empty/`null` filter, // which is always-true above). It compiles to a constant independent of @@ -85,7 +85,7 @@ export const FilterCompilers = { // also the primary path for boolean, whose compiler has no switch and // would otherwise route `none` through the null/false truth-table. if (c.operator === 'none') { - return noneToSQL(c.not); + return noneToSQL(c.not, d); } if (t === 'string' && isStringFilter(c)) { return FilterCompilers.stringCompile(c, x, d); @@ -135,7 +135,7 @@ export const FilterCompilers = { case 'null': return nc.not ? `${x} IS NOT NULL` : `${x} IS NULL`; case 'none': - return noneToSQL(nc.not); + return noneToSQL(nc.not, d); case '()': { const wrapped = '(' + FilterCompilers.numberCompile(nc.expr, x, d) + ')'; @@ -170,8 +170,24 @@ export const FilterCompilers = { // Normally intercepted by compile()'s top-level guard; handled here too so // booleanCompile is self-safe (this if-chain has no default, and `none` // must not fall through to the null/false truth-table below). - return noneToSQL(bc.not); - } else if (bc.operator === '=true') { + return noneToSQL(bc.not, d); + } + if (d.booleanType === 'none') { + // x is a condition, and the same truth table is written in conditions: + // an unknown condition already tests as false, and its negation as true + if (bc.operator === '=true') { + return bc.not ? `NOT ${px}` : x; + } else if (bc.operator === '=false') { + return bc.not ? x : `NOT ${px}`; + } else if (bc.operator === 'null') { + const v = d.sqlConditionAsValue(x); + return bc.not ? `${v} IS NOT NULL` : `${v} IS NULL`; + } else if (bc.operator === 'true') { + return bc.not ? d.sqlNot(x) : x; + } + return bc.not ? x : d.sqlNot(x); + } + if (bc.operator === '=true') { return bc.not ? `NOT ${px}` : x; } else if (bc.operator === '=false') { return bc.not ? x : `NOT ${px}`; @@ -206,7 +222,7 @@ export const FilterCompilers = { case 'null': return sc.not ? `${x} IS NOT NULL` : `${x} IS NULL`; case 'none': - return noneToSQL(sc.not); + return noneToSQL(sc.not, d); case 'empty': return sc.not ? `COALESCE(${x},'') != ''` : `COALESCE(${x},'') = ''`; case '=': { @@ -317,7 +333,7 @@ export const FilterCompilers = { } } if ((includeEmpty && excludeEmpty) || (includeNull && excludeNull)) { - return 'false'; + return d.sqlBoolean(false); } let includeSQL = ''; if (includes.length > 0 || includeNull || includeEmpty) { @@ -352,7 +368,7 @@ export const FilterCompilers = { ? `(${includeSQL}) AND (${excludeSQL})` : includeSQL; } - return excludeSQL !== '' ? excludeSQL : 'true'; + return excludeSQL !== '' ? excludeSQL : d.sqlBoolean(true); } } }, @@ -522,7 +538,7 @@ export class TemporalFilterCompiler { case 'null': return tc.not ? `${x} IS NOT NULL` : `${x} IS NULL`; case 'none': - return noneToSQL(tc.not); + return noneToSQL(tc.not, this.d); case '()': { const wrapped = '(' + this.compile(tc.expr) + ')'; return tc.not ? `NOT ${wrapped}` : wrapped; diff --git a/packages/malloy/src/model/malloy_types.ts b/packages/malloy/src/model/malloy_types.ts index 45ffe45f0..06ee212a2 100644 --- a/packages/malloy/src/model/malloy_types.ts +++ b/packages/malloy/src/model/malloy_types.ts @@ -429,6 +429,9 @@ export interface GenericSQLExpr extends ExprWithKids { node: 'genericSQLExpr'; kids: {args: Expr[]}; src: string[]; + // SQL the model's author wrote (a sql_ function), as opposed to a + // function's implementation + authored?: boolean; } export interface NullNode extends ExprLeaf { diff --git a/packages/malloy/src/model/query_query.ts b/packages/malloy/src/model/query_query.ts index a559e8039..1fec0cb49 100644 --- a/packages/malloy/src/model/query_query.ts +++ b/packages/malloy/src/model/query_query.ts @@ -590,7 +590,13 @@ export class QueryQuery extends QueryField { (which === 'having' && expressionIsCalculation(cond.expressionType)) || (which === 'where' && expressionIsScalar(cond.expressionType)) ) { - const sqlClause = exprToSQL(resultStruct, context, cond.e, undefined); + const sqlClause = exprToSQL( + resultStruct, + context, + cond.e, + undefined, + 'condition' + ); resultFilters.add(sqlClause); } } diff --git a/test/src/databases/all/expr.spec.ts b/test/src/databases/all/expr.spec.ts index b6c905c40..f0318aa55 100644 --- a/test/src/databases/all/expr.spec.ts +++ b/test/src/databases/all/expr.spec.ts @@ -527,12 +527,13 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => { source: a is ${databaseName}.table('malloytest.aircraft_models') extend { where: aircraft_model_code ? '0270202' } run: a -> { - group_by: boolean_1 is sql_boolean("\${seats} > 20") - group_by: boolean_2 is sql_boolean("\${engines} = 2") + group_by: + boolean_1 is pick 'yes' when sql_boolean("\${seats} > 20") else 'no' + boolean_2 is pick 'yes' when sql_boolean("\${engines} = 2") else 'no' } `).toMatchResult(testModel, { - boolean_1: true, - boolean_2: false, + boolean_1: 'yes', + boolean_2: 'no', }); }); @@ -1067,12 +1068,12 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => { dimension: fot is (false) or (true) } -> { select: - no_paren is false and fot - paren is false and (fot) + no_paren is pick 'yes' when false and fot else 'no' + paren is pick 'yes' when false and (fot) else 'no' }` ).toMatchResult(testModel, { - paren: false, - no_paren: false, + paren: 'no', + no_paren: 'no', }); }); }); diff --git a/test/src/databases/all/functions.spec.ts b/test/src/databases/all/functions.spec.ts index 98fe0554d..2e038d4b8 100644 --- a/test/src/databases/all/functions.spec.ts +++ b/test/src/databases/all/functions.spec.ts @@ -51,8 +51,6 @@ expressionModels.forEach((x, databaseName) => { const expressionModel = x.expressionModel; const runtime = x.runtime; const testModel = wrapTestModel(runtime, modelText(databaseName)); - const dbTrue = runtime.dialect.resultBoolean(true); - const dbFalse = runtime.dialect.resultBoolean(false); const funcTestGeneral = async ( expr: string, type: 'group_by' | 'aggregate', @@ -124,14 +122,6 @@ expressionModels.forEach((x, databaseName) => { await funcTestMultiple( ["concat('foo', 'bar')", 'foobar'], ["concat(1, 'bar')", '1bar'], - [ - "concat('cons', true)", - databaseName === 'postgres' - ? 'const' - : databaseName === 'mysql' - ? 'cons1' - : 'construe', - ], ["concat('foo', @2003)", 'foo2003-01-01'], [ "concat('foo', @2003-01-01 12:00:00)", @@ -142,6 +132,20 @@ expressionModels.forEach((x, databaseName) => { ['concat()', ''] ); }); + it.when( + runtime.dialect.booleanType !== 'none' && + !brokenIn('trino', databaseName) && + !brokenIn('presto', databaseName) + )(`works with a boolean - ${databaseName}`, async () => { + await funcTest( + "concat('cons', true)", + databaseName === 'postgres' + ? 'const' + : databaseName === 'mysql' + ? 'cons1' + : 'construe' + ); + }); }); describe('round', () => { @@ -624,16 +628,13 @@ expressionModels.forEach((x, databaseName) => { # test.debug run: state_facts -> { group_by: state - calculate: lag_val is lag(@2011-11-11 11:11:11, 1, now).year = now.year + calculate: lag_val is + pick 'now' when lag(@2011-11-11 11:11:11, 1, now).year = now.year else 'then' }` ) .run(); - expect(result.data.path(0, 'lag_val').value).toBe( - runtime.dialect.resultBoolean(true) - ); - expect(result.data.path(1, 'lag_val').value).toBe( - runtime.dialect.resultBoolean(false) - ); + expect(result.data.path(0, 'lag_val').value).toBe('now'); + expect(result.data.path(1, 'lag_val').value).toBe('then'); }); }); @@ -855,32 +856,45 @@ expressionModels.forEach((x, databaseName) => { : "'+inf'::number"; it.when(databaseName !== 'mysql')(`works - ${databaseName}`, async () => { await funcTestMultiple( - [`is_inf(${inf})`, dbTrue], - ['is_inf(100)', dbFalse], - ['is_inf(null)', dbFalse] + [`pick 'yes' when is_inf(${inf}) else 'no'`, 'yes'], + ["pick 'yes' when is_inf(100) else 'no'", 'no'], + ["pick 'yes' when is_inf(null) else 'no'", 'no'] ); }); + // A null input is false, not null + it.when(databaseName !== 'mysql' && runtime.dialect.booleanType !== 'none')( + `is false for null - ${databaseName}`, + async () => { + await funcTest('is_inf(null)', runtime.dialect.resultBoolean(false)); + } + ); }); describe('is_nan', () => { it.when(databaseName !== 'mysql')(`works - ${databaseName}`, async () => { await funcTestMultiple( - ["is_nan('NaN'::number)", dbTrue], - ['is_nan(100)', dbFalse], - ['is_nan(null)', dbFalse] + ["pick 'yes' when is_nan('NaN'::number) else 'no'", 'yes'], + ["pick 'yes' when is_nan(100) else 'no'", 'no'], + ["pick 'yes' when is_nan(null) else 'no'", 'no'] ); }); + it.when(databaseName !== 'mysql' && runtime.dialect.booleanType !== 'none')( + `is false for null - ${databaseName}`, + async () => { + await funcTest('is_nan(null)', runtime.dialect.resultBoolean(false)); + } + ); }); describe('greatest', () => { it(`works - ${databaseName}`, async () => { await funcTestMultiple( ['greatest(1, 10, -100)', 10], [ - 'greatest(@2003, @2004, @1994) = @2004', - runtime.dialect.resultBoolean(true), + "pick 'yes' when greatest(@2003, @2004, @1994) = @2004 else 'no'", + 'yes', ], [ - 'greatest(@2023-05-26 11:58:00, @2023-05-26 11:59:00) = @2023-05-26 11:59:00', - runtime.dialect.resultBoolean(true), + "pick 'yes' when greatest(@2023-05-26 11:58:00, @2023-05-26 11:59:00) = @2023-05-26 11:59:00 else 'no'", + 'yes', ], ["greatest('a', 'b')", 'b'], ['greatest(1, null, 0)', null], @@ -892,13 +906,10 @@ expressionModels.forEach((x, databaseName) => { it(`works - ${databaseName}`, async () => { await funcTestMultiple( ['least(1, 10, -100)', -100], + ["pick 'yes' when least(@2003, @2004, @1994) = @1994 else 'no'", 'yes'], [ - 'least(@2003, @2004, @1994) = @1994', - runtime.dialect.resultBoolean(true), - ], - [ - 'least(@2023-05-26 11:58:00, @2023-05-26 11:59:00) = @2023-05-26 11:58:00', - runtime.dialect.resultBoolean(true), + "pick 'yes' when least(@2023-05-26 11:58:00, @2023-05-26 11:59:00) = @2023-05-26 11:58:00 else 'no'", + 'yes', ], ["least('a', 'b')", 'a'], ['least(1, null, 0)', null], @@ -929,36 +940,48 @@ expressionModels.forEach((x, databaseName) => { it(`works - ${databaseName}`, async () => { await funcTestMultiple( [ - "starts_with('hello world', 'hello')", - runtime.dialect.resultBoolean(true), + "pick 'yes' when starts_with('hello world', 'hello') else 'no'", + 'yes', ], - [ - "starts_with('hello world', 'world')", - runtime.dialect.resultBoolean(false), - ], - ["starts_with(null, 'world')", runtime.dialect.resultBoolean(false)], - [ - "starts_with('hello world', null)", - runtime.dialect.resultBoolean(false), - ] + ["pick 'yes' when starts_with('hello world', 'world') else 'no'", 'no'], + ["pick 'yes' when starts_with(null, 'world') else 'no'", 'no'], + ["pick 'yes' when starts_with('hello world', null) else 'no'", 'no'] ); }); + it.when(runtime.dialect.booleanType !== 'none')( + `is false for null - ${databaseName}`, + async () => { + await funcTestMultiple( + ["starts_with(null, 'world')", runtime.dialect.resultBoolean(false)], + [ + "starts_with('hello world', null)", + runtime.dialect.resultBoolean(false), + ] + ); + } + ); }); describe('ends_with', () => { it(`works - ${databaseName}`, async () => { await funcTestMultiple( - [ - "ends_with('hello world', 'world')", - runtime.dialect.resultBoolean(true), - ], - [ - "ends_with('hello world', 'hello')", - runtime.dialect.resultBoolean(false), - ], - ["ends_with(null, 'world')", runtime.dialect.resultBoolean(false)], - ["ends_with('hello world', null)", runtime.dialect.resultBoolean(false)] + ["pick 'yes' when ends_with('hello world', 'world') else 'no'", 'yes'], + ["pick 'yes' when ends_with('hello world', 'hello') else 'no'", 'no'], + ["pick 'yes' when ends_with(null, 'world') else 'no'", 'no'], + ["pick 'yes' when ends_with('hello world', null) else 'no'", 'no'] ); }); + it.when(runtime.dialect.booleanType !== 'none')( + `is false for null - ${databaseName}`, + async () => { + await funcTestMultiple( + ["ends_with(null, 'world')", runtime.dialect.resultBoolean(false)], + [ + "ends_with('hello world', null)", + runtime.dialect.resultBoolean(false), + ] + ); + } + ); }); describe('trim', () => { it(`trim works - ${databaseName}`, async () => { @@ -1001,14 +1024,17 @@ expressionModels.forEach((x, databaseName) => { // There are around a billion values that rand() can be, so if this // test fails, most likely something is broken. Otherwise, you're the lucky // one in a billion! - await funcTest('rand() = rand()', runtime.dialect.resultBoolean(false)); + await funcTest( + "pick 'same' when rand() = rand() else 'differ'", + 'differ' + ); }); }); describe('pi', () => { it(`is pi - ${databaseName}`, async () => { await funcTest( - 'abs(pi() - 3.141592653589793) < 0.0000000000001', - runtime.dialect.resultBoolean(true) + "pick 'yes' when abs(pi() - 3.141592653589793) < 0.0000000000001 else 'no'", + 'yes' ); }); }); diff --git a/test/src/databases/all/new-matchers.spec.ts b/test/src/databases/all/new-matchers.spec.ts index 56d3af3d6..fa3abe318 100644 --- a/test/src/databases/all/new-matchers.spec.ts +++ b/test/src/databases/all/new-matchers.spec.ts @@ -11,8 +11,11 @@ import '@malloydata/malloy/test/matchers'; const runtimes = new RuntimeList(databasesFromEnvironmentOr(allDatabases)); describe.each(runtimes.runtimeList)('New matchers for %s', (db, runtime) => { + // A dialect without boolean values has no boolean column + const booleans = runtime.dialect.booleanType !== 'none'; + describe('mkTestModel and TV', () => { - test('basic types with type inference', async () => { + test.when(booleans)('basic types with type inference', async () => { const tm = mkTestModel(runtime, { data: [ {t_int: 1, t_string: 'a', t_bool: true}, @@ -155,16 +158,19 @@ describe.each(runtimes.runtimeList)('New matchers for %s', (db, runtime) => { }); }); - test('plain booleans work with dialect boolean handling', async () => { - const tm = mkTestModel(runtime, { - data: [{a: true, b: false}], - }); - // Booleans are handled by dialect.resultBoolean() in the matcher - await expect('run: data -> { select: * }').toMatchResult(tm, { - a: true, - b: false, - }); - }); + test.when(booleans)( + 'plain booleans work with dialect boolean handling', + async () => { + const tm = mkTestModel(runtime, { + data: [{a: true, b: false}], + }); + // Booleans are handled by dialect.resultBoolean() in the matcher + await expect('run: data -> { select: * }').toMatchResult(tm, { + a: true, + b: false, + }); + } + ); }); describe('test.debug tag', () => { diff --git a/test/src/databases/all/orderby.spec.ts b/test/src/databases/all/orderby.spec.ts index 88bc7c847..83eb294fa 100644 --- a/test/src/databases/all/orderby.spec.ts +++ b/test/src/databases/all/orderby.spec.ts @@ -24,20 +24,26 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => { }` ); - it(`boolean type - ${databaseName}`, async () => { - await expect(` + // A dialect without boolean values cannot group by one + it.when(runtime.dialect.booleanType !== 'none')( + `boolean type - ${databaseName}`, + async () => { + await expect(` run: models-> { group_by: big is seats >=20 aggregate: model_count is count() } `).toMatchResult(orderByModel, { - big: false, - model_count: 58451, - }); - }); + big: false, + model_count: 58451, + }); + } + ); - it(`boolean in pipeline - ${databaseName}`, async () => { - await expect(` + it.when(runtime.dialect.booleanType !== 'none')( + `boolean in pipeline - ${databaseName}`, + async () => { + await expect(` run: models->{ group_by: manufacturer, @@ -48,10 +54,11 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => { aggregate: model_count is model_count.sum() } `).toMatchResult(orderByModel, { - big: false, - model_count: 58500, - }); - }); + big: false, + model_count: 58500, + }); + } + ); it(`filtered measures in model are aggregates #352 - ${databaseName}`, async () => { await expect(` diff --git a/test/src/databases/all/test-select.spec.ts b/test/src/databases/all/test-select.spec.ts index 142a5d1ad..50adc5150 100644 --- a/test/src/databases/all/test-select.spec.ts +++ b/test/src/databases/all/test-select.spec.ts @@ -15,8 +15,11 @@ describe.each(runtimes.runtimeList)('TestSelect for %s', (db, runtime) => { const ts = new TestSelect(runtime.dialect); const testModel = wrapTestModel(runtime, ''); + // A dialect without boolean values has no boolean column + const booleans = runtime.dialect.booleanType !== 'none'; + // Basic Type Tests - test(`${db} inferred basic types`, async () => { + test.when(booleans)(`${db} inferred basic types`, async () => { const sql = ts.generate( {t_int: 1, t_string: 'a', t_bool: true, t_float: 1.5}, {t_int: 2, t_string: 'b', t_bool: false, t_float: 2.5} @@ -27,7 +30,7 @@ describe.each(runtimes.runtimeList)('TestSelect for %s', (db, runtime) => { ]); }); - test(`${db} explicit type hints`, async () => { + test.when(booleans)(`${db} explicit type hints`, async () => { const sql = ts.generate({ t_int: ts.mk_int(1), t_float: ts.mk_float(1.5), diff --git a/test/src/databases/all/time.spec.ts b/test/src/databases/all/time.spec.ts index 300e171a0..2f7254d02 100644 --- a/test/src/databases/all/time.spec.ts +++ b/test/src/databases/all/time.spec.ts @@ -337,13 +337,13 @@ describe.each(runtimes.runtimeList)('%s date and time', (dbName, runtime) => { run: tzdata -> { select: ts is tstz::timestamp - same is tstz::timestamp = tstz - back is same_instant::timestamptz = tstz + same is pick 'yes' when tstz::timestamp = tstz else 'no' + back is pick 'yes' when same_instant::timestamptz = tstz else 'no' } `).toMatchResult(tzModel(), { ts: '2020-01-31T02:00:00.000Z', - same: true, - back: true, + same: 'yes', + back: 'yes', }); }); @@ -351,16 +351,16 @@ describe.each(runtimes.runtimeList)('%s date and time', (dbName, runtime) => { await expect(` run: tzdata -> { select: - eq_instant is tstz = same_instant - eq_digits is tstz = same_digits - gt_digits is tstz > same_digits - lt_digits is same_digits < tstz + eq_instant is pick 'yes' when tstz = same_instant else 'no' + eq_digits is pick 'yes' when tstz = same_digits else 'no' + gt_digits is pick 'yes' when tstz > same_digits else 'no' + lt_digits is pick 'yes' when same_digits < tstz else 'no' } `).toMatchResult(tzModel(), { - eq_instant: true, - eq_digits: false, - gt_digits: true, - lt_digits: true, + eq_instant: 'yes', + eq_digits: 'no', + gt_digits: 'yes', + lt_digits: 'yes', }); }); }); @@ -600,16 +600,16 @@ describe.each(runtimes.runtimeList)('%s date and time', (dbName, runtime) => { }); await expect(` run: fourTimes -> { select: - isBefore is tbefore ? @2026-01-01 - isFirst is tfirst ? @2026-01-01 - isLast is tlast ? @2026-01-01 - isAfter is tafter ? @2026-01-01 + isBefore is pick 'in' when tbefore ? @2026-01-01 else 'out' + isFirst is pick 'in' when tfirst ? @2026-01-01 else 'out' + isLast is pick 'in' when tlast ? @2026-01-01 else 'out' + isAfter is pick 'in' when tafter ? @2026-01-01 else 'out' } `).toMatchResult(tm, { - isBefore: false, - isFirst: true, - isLast: true, - isAfter: false, + isBefore: 'out', + isFirst: 'in', + isLast: 'in', + isAfter: 'out', }); }); }); @@ -940,8 +940,8 @@ describe.each(runtimes.runtimeList)('%s: query tz', (dbName, runtime) => { ], }); await expect( - 'run: tzdata -> { select: cmp is updated ? @2020-01 }' - ).toMatchResult(tzModel, {cmp: true}); + "run: tzdata -> { select: cmp is pick 'in' when updated ? @2020-01 else 'out' }" + ).toMatchResult(tzModel, {cmp: 'in'}); }); // 11:00 on the 31st in Tokyo is 20:00 on the 30th in Mexico City. With a @@ -1003,13 +1003,13 @@ describe.each(runtimes.runtimeList)('%s: query tz', (dbName, runtime) => { timezone: '${zone}' select: ts is tstz::timestamp - same is tstz::timestamp = tstz - back is same_instant::timestamptz = tstz + same is pick 'yes' when tstz::timestamp = tstz else 'no' + back is pick 'yes' when same_instant::timestamptz = tstz else 'no' } `).toMatchResult(tzModel(), { ts: '2020-01-31T02:00:00.000Z', - same: true, - back: true, + same: 'yes', + back: 'yes', }); }); }); @@ -1092,16 +1092,16 @@ describe.each(runtimes.runtimeList)('%s: query tz', (dbName, runtime) => { timezone: 'Europe/Dublin' select: june1_correct, - month_ok is by_month = june1_correct, by_month - day_ok is by_day = june1_correct, by_day - hour_ok is by_hour = june1_correct, by_hour - quarter_ok is by_quarter = august1_correct, by_quarter + month_ok is pick 'yes' when by_month = june1_correct else 'no', by_month + day_ok is pick 'yes' when by_day = june1_correct else 'no', by_day + hour_ok is pick 'yes' when by_hour = june1_correct else 'no', by_hour + quarter_ok is pick 'yes' when by_quarter = august1_correct else 'no', by_quarter }` ).toMatchResult(testModel, { - month_ok: true, - day_ok: true, - hour_ok: true, - quarter_ok: true, + month_ok: 'yes', + day_ok: 'yes', + hour_ok: 'yes', + quarter_ok: 'yes', }); }); }); diff --git a/test/src/databases/sqlserver/boolean-coercion.spec.ts b/test/src/databases/sqlserver/boolean-coercion.spec.ts new file mode 100644 index 000000000..30b8c1580 --- /dev/null +++ b/test/src/databases/sqlserver/boolean-coercion.spec.ts @@ -0,0 +1,100 @@ +/* + * Copyright Contributors to the Malloy project + * SPDX-License-Identifier: MIT + */ + +import {RuntimeList} from '../../runtimes'; +import {describeIfDatabaseAvailable} from '../../util'; +import '@malloydata/malloy/test/matchers'; +import {wrapTestModel} from '@malloydata/malloy/test'; + +const [describe] = describeIfDatabaseAvailable(['sqlserver']); + +// In the boolean_coercion experiment a boolean used as a value is 1, 0 or +// NULL in SQL, which the result reader shows as a boolean, and a boolean +// column read back is a condition again. +describe('SQL Server boolean coercion', () => { + const runtimes = new RuntimeList(['sqlserver']); + const runtime = runtimes.runtimeMap.get('sqlserver'); + if (runtime === undefined) { + throw new Error("Couldn't build runtime"); + } + const tm = wrapTestModel( + runtime, + `##! experimental { sql_functions boolean_coercion } +source: s is sqlserver.sql("SELECT 1 AS n UNION ALL SELECT 2 UNION ALL SELECT 2") extend { + dimension: big is n > 1 + measure: c is count() +} +source: sn is sqlserver.sql("SELECT CAST(NULL AS INT) AS n UNION ALL SELECT 2") extend { + dimension: big is n > 1 + measure: c is count() +} +source: pk is sqlserver.sql("SELECT 1 AS n UNION ALL SELECT 2") extend { + dimension: big is n > 1 + primary_key: big +} +source: j is sqlserver.sql("SELECT 1 AS n UNION ALL SELECT 2") extend { + join_one: pk on pk.n = n +} +` + ); + + afterAll(async () => { + await runtimes.closeAll(); + }); + + test('a boolean result column', async () => { + await expect(` + run: s -> { group_by: big; aggregate: c; order_by: big } + `).toEqualResult(tm, [ + {big: false, c: 1}, + {big: true, c: 2}, + ]); + }); + + test('an unknown condition is NULL', async () => { + await expect(` + run: sn -> { group_by: big; aggregate: c; order_by: big } + `).toEqualResult(tm, [ + {big: null, c: 1}, + {big: true, c: 1}, + ]); + }); + + test('select: * includes the boolean', async () => { + await expect(` + run: s -> { select: *; where: n = 1 } + `).toEqualResult(tm, [{n: 1, big: false}]); + }); + + test('a boolean primary key', async () => { + await expect(` + run: j -> { aggregate: keys is pk.count(), total is pk.n.sum() } + `).toEqualResult(tm, [{keys: 2, total: 3}]); + }); + + test('a boolean column from an earlier stage is a condition again', async () => { + await expect(` + run: s -> { group_by: n, big } -> { where: big; select: n } + `).toEqualResult(tm, [{n: 2}]); + }); + + test('a boolean compared and null-tested', async () => { + await expect(` + run: sn -> { aggregate: nulls is c { where: big is null }, trues is c { where: big = true } } + `).toEqualResult(tm, [{nulls: 1, trues: 1}]); + }); + + test('a condition is still a condition', async () => { + await expect(` + run: s -> { where: big; aggregate: c } + `).toEqualResult(tm, [{c: 2}]); + await expect(` + run: s -> { where: not big; aggregate: c } + `).toEqualResult(tm, [{c: 1}]); + await expect(` + run: sn -> { where: not big; aggregate: c } + `).toEqualResult(tm, [{c: 1}]); + }); +}); diff --git a/test/src/databases/sqlserver/sqlserver.spec.ts b/test/src/databases/sqlserver/sqlserver.spec.ts index 9a07107d6..9a4a4f07b 100644 --- a/test/src/databases/sqlserver/sqlserver.spec.ts +++ b/test/src/databases/sqlserver/sqlserver.spec.ts @@ -94,23 +94,82 @@ describe('SQL Server', () => { `).toEqualResult(tm, [{n: 51}]); }); - test('a declared boolean is a 0/1 value', async () => { + test('a declared boolean in a result column is refused, naming the value to write', async () => { + await expect( + runtime + .loadQuery( + `run: sqlserver.table('malloytest.state_facts') extend { + dimension: big is airport_count > 500 + } -> { group_by: big; aggregate: n is count() }` + ) + .run() + ).rejects.toThrow( + "'sqlserver' does not support a boolean as a result column; write the value, pick 'yes' when big else 'no'" + ); + }); + + test('a declared boolean is a condition in where: and having:', async () => { + await expect(` + run: sqlserver.table('malloytest.state_facts') extend { + dimension: big is airport_count > 500 + measure: n is count() + } -> { + where: big + aggregate: n + having: n > 1 + } + `).toEqualResult(tm, [{n: 12}]); + }); + + test('not, and a comparison true on NULL, follow Malloy rather than SQL', async () => { + await expect(` + run: sqlserver.sql('SELECT CAST(NULL AS INT) AS n, 1 AS one') -> { + select: + not_null is pick 'yes' when not (n = 1) else 'no' + ne_null is pick 'yes' when n != 1 else 'no' + not_one is pick 'yes' when not (one = 1) else 'no' + ne_one is pick 'yes' when one != 1 else 'no' + } + `).toEqualResult(tm, [ + {not_null: 'yes', ne_null: 'yes', not_one: 'no', ne_one: 'no'}, + ]); + }); + + test('a comparison picks', async () => { await expect(` run: sqlserver.table('malloytest.state_facts') -> { - group_by: - big is airport_count > 500 - c_state is starts_with(state, 'C') + group_by: size is pick 'big' when airport_count > 500 else 'small' aggregate: n is count() - order_by: big desc, c_state desc + order_by: size } `).toEqualResult(tm, [ - {big: true, c_state: true, n: 1}, - {big: true, c_state: false, n: 11}, - {big: false, c_state: true, n: 2}, - {big: false, c_state: false, n: 37}, + {size: 'big', n: 12}, + {size: 'small', n: 39}, ]); }); + test('a boolean function is a condition', async () => { + await expect(` + run: sqlserver.table('malloytest.state_facts') -> { + where: starts_with(state, 'C') and not ends_with(state, 'A') + select: state + order_by: state + } + `).toEqualResult(tm, [{state: 'CO'}, {state: 'CT'}]); + }); + + test('sql_boolean is a condition', async () => { + await expect(` + ##! experimental { sql_functions } + run: sqlserver.table('malloytest.state_facts') extend { + dimension: big is sql_boolean("\${airport_count} > 500") + } -> { + where: big + aggregate: n is count() + } + `).toEqualResult(tm, [{n: 12}]); + }); + test('a cast to boolean is refused', async () => { await expect( runtime @@ -123,7 +182,7 @@ describe('SQL Server', () => { ).rejects.toThrow(/boolean/); }); - test('starts_with and ends_with are conditions', async () => { + test('starts_with, ends_with and is_nan filter an aggregate', async () => { await expect(` run: sqlserver.table('malloytest.state_facts') -> { aggregate: @@ -211,13 +270,13 @@ describe('SQL Server', () => { test('a constant dimension groups with the others', async () => { await expect(` run: sqlserver.table('malloytest.state_facts') -> { - group_by: tag is 'facts', big is airport_count > 500 + group_by: tag is 'facts', size is pick 'big' when airport_count > 500 else 'small' aggregate: n is count() - order_by: big desc + order_by: size } `).toEqualResult(tm, [ - {tag: 'facts', big: true, n: 12}, - {tag: 'facts', big: false, n: 39}, + {tag: 'facts', size: 'big', n: 12}, + {tag: 'facts', size: 'small', n: 39}, ]); });