feat: add HandleEmtpyInListsPlugin. (#925)

* feat: empty where in plugin

* test: add new tests

* chore: remove unneccesary typeguards

* fix: change to binary operator node

* test: update tests to do both in and not in

* test: for having

* chore: rm test

* test: nullable tests

* chore: nit

* chore: condense suite

* chore: db config override

* chore: extra console log

* chore: empty arr plugin docs

* HandleEmptyInListsPlugin initial commit.

Co-authored-by: Austin Woon Quan <[email protected]>

---------

Co-authored-by: Austin Woon <[email protected]>
Co-authored-by: igalklebanov <[email protected]>

site/docs/plugins.md

@@ -20,4 +20,8 @@ A plugin that converts snake_case identifiers in the database into camelCase in
 
 ### Deduplicate joins plugin
 
-Plugin that removes duplicate joins from queries. You can read more about it in the [examples](/docs/recipes/deduplicate-joins) section or check the [API docs](https://kysely-org.github.io/kysely-apidoc/classes/DeduplicateJoinsPlugin.html).
+A plugin that removes duplicate joins from queries. You can read more about it in the [examples](/docs/recipes/deduplicate-joins) section or check the [API docs](https://kysely-org.github.io/kysely-apidoc/classes/DeduplicateJoinsPlugin.html).
+
+### Handle `in ()` and `not in ()` plugin
+
+A plugin that allows handling `in ()` and `not in ()` with a chosen strategy. [Learn more](https://kysely-org.github.io/kysely-apidoc/classes/HandleEmptyWhereInListsPlugin.html).

src/index.ts

@@ -110,6 +110,8 @@ export * from './plugin/camel-case/camel-case-plugin.js'
 export * from './plugin/deduplicate-joins/deduplicate-joins-plugin.js'
 export * from './plugin/with-schema/with-schema-plugin.js'
 export * from './plugin/parse-json-results/parse-json-results-plugin.js'
+export * from './plugin/handle-empty-in-lists/handle-empty-in-lists-plugin.js'
+export * from './plugin/handle-empty-in-lists/handle-empty-in-lists.js'
 
 export * from './operation-node/add-column-node.js'
 export * from './operation-node/add-constraint-node.js'

src/plugin/handle-empty-in-lists/handle-empty-in-lists-plugin.ts

@@ -0,0 +1,171 @@
+import { QueryResult } from '../../driver/database-connection.js'
+import { RootOperationNode } from '../../query-compiler/query-compiler.js'
+import {
+  KyselyPlugin,
+  PluginTransformQueryArgs,
+  PluginTransformResultArgs,
+} from '../kysely-plugin.js'
+import { UnknownRow } from '../../util/type-utils.js'
+import { HandleEmptyInListsTransformer } from './handle-empty-in-lists-transformer.js'
+import { HandleEmptyInListsOptions } from './handle-empty-in-lists.js'
+
+/**
+ * A plugin that allows handling `in ()` and `not in ()` expressions.
+ *
+ * These expressions are invalid SQL syntax for many databases, and result in runtime
+ * database errors.
+ *
+ * The workarounds used by other libraries always involve modifying the query under
+ * the hood, which is not aligned with Kysely's philosophy of WYSIWYG. We recommend manually checking
+ * for empty arrays before passing them as arguments to `in` and `not in` expressions
+ * instead, but understand that this can be cumbersome. Hence we're going with an
+ * opt-in approach where you can choose if and how to handle these cases. We do
+ * not want to make this the default behavior, as it can lead to unexpected behavior.
+ * Use it at your own risk. Test it. Make sure it works as expected for you.
+ *
+ * Using this plugin also allows you to throw an error (thus avoiding unnecessary
+ * requests to the database) or print a warning in these cases.
+ *
+ * ### Examples
+ *
+ * The following strategy replaces the `in`/`not in` expression with a noncontingent
+ * expression. A contradiction (falsy) `1 = 0` for `in`, and a tautology (truthy) `1 = 1` for `not in`),
+ * similarily to how {@link https://github.com/knex/knex/blob/176151d8048b2a7feeb89a3d649a5580786d4f4e/docs/src/guide/query-builder.md#L1763 | Knex.js},
+ * {@link https://github.com/prisma/prisma-engines/blob/99168c54187178484dae45d9478aa40cfd1866d2/quaint/src/visitor.rs#L804-L823 | PrismaORM},
+ * {@link https://github.com/laravel/framework/blob/8.x/src/Illuminate/Database/Query/Grammars/Grammar.php#L284-L291 | Laravel},
+ * {@link https://docs.sqlalchemy.org/en/13/core/engines.html#sqlalchemy.create_engine.params.empty_in_strategy | SQLAlchemy}
+ * handle this.
+ *
+ * ```ts
+ * import Sqlite from 'better-sqlite3'
+ * import {
+ *   HandleEmptyInListsPlugin,
+ *   Kysely,
+ *   replaceWithNoncontingentExpression,
+ *   SqliteDialect,
+ * } from 'kysely'
+ * import type { Database } from 'type-editor' // imaginary module
+ *
+ * const db = new Kysely<Database>({
+ *   dialect: new SqliteDialect({
+ *     database: new Sqlite(':memory:'),
+ *   }),
+ *   plugins: [
+ *     new HandleEmptyInListsPlugin({
+ *       strategy: replaceWithNoncontingentExpression
+ *     })
+ *   ],
+ * })
+ *
+ * const results = await db
+ *   .selectFrom('person')
+ *   .where('id', 'in', [])
+ *   .where('first_name', 'not in', [])
+ *   .selectAll()
+ *   .execute()
+ * ```
+ *
+ * The generated SQL (SQLite):
+ *
+ * ```sql
+ * select * from "person" where 1 = 0 and 1 = 1
+ * ```
+ *
+ * The following strategy does the following:
+ *
+ * When `in`, pushes a `null` value into the empty list resulting in `in (null)`,
+ * similiarly to how {@link https://github.com/typeorm/typeorm/blob/0280cdc451c35ef73c830eb1191c95d34f6ce06e/src/query-builder/QueryBuilder.ts#L919-L922 | TypeORM}
+ * and {@link https://github.com/sequelize/sequelize/blob/0f2891c6897e12bf9bf56df344aae5b698f58c7d/packages/core/src/abstract-dialect/where-sql-builder.ts#L368-L379 | Sequelize}
+ * handle `in ()`. `in (null)` is logically the equivalent of `= null`, which returns
+ * `null`, which is a falsy expression in most SQL databases. We recommend NOT
+ * using this strategy if you plan to use `in` in `select`, `returning`, or `output`
+ * clauses, as the return type differs from the `SqlBool` default type for comparisons.
+ *
+ * When `not in`, casts the left operand as `char` and pushes a unique value into
+ * the empty list resulting in `cast({{lhs}} as char) not in ({{VALUE}})`. Casting
+ * is required to avoid database errors with non-string values.
+ *
+ * ```ts
+ * import Sqlite from 'better-sqlite3'
+ * import {
+ *   HandleEmptyInListsPlugin,
+ *   Kysely,
+ *   pushValueIntoList,
+ *   SqliteDialect
+ * } from 'kysely'
+ * import type { Database } from 'type-editor' // imaginary module
+ *
+ * const db = new Kysely<Database>({
+ *   dialect: new SqliteDialect({
+ *     database: new Sqlite(':memory:'),
+ *   }),
+ *   plugins: [
+ *     new HandleEmptyInListsPlugin({
+ *       strategy: pushValueIntoList('__kysely_no_values_were_provided__') // choose a unique value for not in. has to be something with zero chance being in the data.
+ *     })
+ *   ],
+ * })
+ *
+ * const results = await db
+ *   .selectFrom('person')
+ *   .where('id', 'in', [])
+ *   .where('first_name', 'not in', [])
+ *   .selectAll()
+ *   .execute()
+ * ```
+ *
+ * The generated SQL (SQLite):
+ *
+ * ```sql
+ * select * from "person" where "id" in (null) and cast("first_name" as char) not in ('__kysely_no_values_were_provided__')
+ * ```
+ *
+ * The following custom strategy throws an error when an empty list is encountered
+ * to avoid unnecessary requests to the database:
+ *
+ * ```ts
+ * import Sqlite from 'better-sqlite3'
+ * import {
+ *   HandleEmptyInListsPlugin,
+ *   Kysely,
+ *   SqliteDialect
+ * } from 'kysely'
+ * import type { Database } from 'type-editor' // imaginary module
+ *
+ * const db = new Kysely<Database>({
+ *   dialect: new SqliteDialect({
+ *     database: new Sqlite(':memory:'),
+ *   }),
+ *   plugins: [
+ *     new HandleEmptyInListsPlugin({
+ *       strategy: () => {
+ *         throw new Error('Empty in/not-in is not allowed')
+ *       }
+ *     })
+ *   ],
+ * })
+ *
+ * const results = await db
+ *   .selectFrom('person')
+ *   .where('id', 'in', [])
+ *   .selectAll()
+ *   .execute() // throws an error with 'Empty in/not-in is not allowed' message!
+ * ```
+ */
+export class HandleEmptyInListsPlugin implements KyselyPlugin {
+  readonly #transformer: HandleEmptyInListsTransformer
+
+  constructor(readonly opt: HandleEmptyInListsOptions) {
+    this.#transformer = new HandleEmptyInListsTransformer(opt.strategy)
+  }
+
+  transformQuery(args: PluginTransformQueryArgs): RootOperationNode {
+    return this.#transformer.transformNode(args.node)
+  }
+
+  async transformResult(
+    args: PluginTransformResultArgs,
+  ): Promise<QueryResult<UnknownRow>> {
+    return args.result
+  }
+}

src/plugin/handle-empty-in-lists/handle-empty-in-lists-transformer.ts

@@ -0,0 +1,40 @@
+import { BinaryOperationNode } from '../../operation-node/binary-operation-node.js'
+import { OperationNodeTransformer } from '../../operation-node/operation-node-transformer.js'
+import { PrimitiveValueListNode } from '../../operation-node/primitive-value-list-node.js'
+import { OperatorNode } from '../../operation-node/operator-node.js'
+import {
+  EmptyInListNode,
+  EmptyInListsStrategy,
+} from './handle-empty-in-lists.js'
+import { ValueListNode } from '../../operation-node/value-list-node.js'
+
+export class HandleEmptyInListsTransformer extends OperationNodeTransformer {
+  readonly #strategy: EmptyInListsStrategy
+
+  constructor(strategy: EmptyInListsStrategy) {
+    super()
+    this.#strategy = strategy
+  }
+
+  protected transformBinaryOperation(
+    node: BinaryOperationNode,
+  ): BinaryOperationNode {
+    if (this.#isEmptyInListNode(node)) {
+      return this.#strategy(node)
+    }
+
+    return node
+  }
+
+  #isEmptyInListNode(node: BinaryOperationNode): node is EmptyInListNode {
+    const { operator, rightOperand } = node
+
+    return (
+      (PrimitiveValueListNode.is(rightOperand) ||
+        ValueListNode.is(rightOperand)) &&
+      rightOperand.values.length === 0 &&
+      OperatorNode.is(operator) &&
+      (operator.operator === 'in' || operator.operator === 'not in')
+    )
+  }
+}

src/plugin/handle-empty-in-lists/handle-empty-in-lists.ts

@@ -0,0 +1,102 @@
+import { BinaryOperationNode } from '../../operation-node/binary-operation-node.js'
+import { CastNode } from '../../operation-node/cast-node.js'
+import { DataTypeNode } from '../../operation-node/data-type-node.js'
+import { OperatorNode } from '../../operation-node/operator-node.js'
+import { ParensNode } from '../../operation-node/parens-node.js'
+import { PrimitiveValueListNode } from '../../operation-node/primitive-value-list-node.js'
+import { ValueListNode } from '../../operation-node/value-list-node.js'
+import { ValueNode } from '../../operation-node/value-node.js'
+import { freeze } from '../../util/object-utils.js'
+
+export interface HandleEmptyInListsOptions {
+  /**
+   * The strategy to use when handling `in ()` and `not in ()`.
+   *
+   * See {@link HandleEmptyInListsPlugin} for examples.
+   */
+  strategy: EmptyInListsStrategy
+}
+
+export type EmptyInListNode = BinaryOperationNode & {
+  operator: OperatorNode & {
+    operator: 'in' | 'not in'
+  }
+  rightOperand: (ValueListNode | PrimitiveValueListNode) & {
+    values: Readonly<[]>
+  }
+}
+
+export type EmptyInListsStrategy = (
+  node: EmptyInListNode,
+) => BinaryOperationNode
+
+let contradiction: BinaryOperationNode
+let eq: OperatorNode
+let one: ValueNode
+let tautology: BinaryOperationNode
+/**
+ * Replaces the `in`/`not in` expression with a noncontingent expression (always true or always
+ * false) depending on the original operator.
+ *
+ * This is how Knex.js, PrismaORM, Laravel, and SQLAlchemy handle `in ()` and `not in ()`.
+ *
+ * See {@link pushValueIntoList} for an alternative strategy.
+ */
+export function replaceWithNoncontingentExpression(
+  node: EmptyInListNode,
+): BinaryOperationNode {
+  const _one = (one ||= ValueNode.createImmediate(1))
+  const _eq = (eq ||= OperatorNode.create('='))
+
+  if (node.operator.operator === 'in') {
+    return (contradiction ||= BinaryOperationNode.create(
+      _one,
+      _eq,
+      ValueNode.createImmediate(0),
+    ))
+  }
+
+  return (tautology ||= BinaryOperationNode.create(_one, _eq, _one))
+}
+
+let char: DataTypeNode
+let listNull: ValueListNode
+let listVal: ValueListNode
+/**
+ * When `in`, pushes a `null` value into the list resulting in `in (null)`. This
+ * is how TypeORM and Sequelize handle `in ()`. `in (null)` is logically the equivalent
+ * of `= null`, which returns `null`, which is a falsy expression in most SQL databases.
+ * We recommend NOT using this strategy if you plan to use `in` in `select`, `returning`,
+ * or `output` clauses, as the return type differs from the `SqlBool` default type.
+ *
+ * When `not in`, casts the left operand as `char` and pushes a literal value into
+ * the list resulting in `cast({{lhs}} as char) not in ({{VALUE}})`. Casting
+ * is required to avoid database errors with non-string columns.
+ *
+ * See {@link replaceWithNoncontingentExpression} for an alternative strategy.
+ */
+export function pushValueIntoList(
+  uniqueNotInLiteral: '__kysely_no_values_were_provided__' | (string & {}),
+): EmptyInListsStrategy {
+  return function pushValueIntoList(node) {
+    if (node.operator.operator === 'in') {
+      return freeze({
+        ...node,
+        rightOperand: (listNull ||= ValueListNode.create([
+          ValueNode.createImmediate(null),
+        ])),
+      })
+    }
+
+    return freeze({
+      ...node,
+      leftOperand: CastNode.create(
+        node.leftOperand,
+        (char ||= DataTypeNode.create('char')),
+      ),
+      rightOperand: (listVal ||= ValueListNode.create([
+        ValueNode.createImmediate(uniqueNotInLiteral),
+      ])),
+    })
+  }
+}

test/node/src/controlled-transaction.test.ts

@@ -44,10 +44,12 @@ for (const dialect of DIALECTS) {
     >
 
     before(async function () {
-      ctx = await initTest(this, dialect, (event) => {
-        if (event.level === 'query') {
-          executedQueries.push(event.query)
-        }
+      ctx = await initTest(this, dialect, {
+        log(event) {
+          if (event.level === 'query') {
+            executedQueries.push(event.query)
+          }
+        },
       })
     })
 

test/node/src/test-setup.ts

@@ -217,15 +217,12 @@ export const DB_CONFIGS: PerDialect<KyselyConfig> = {
 export async function initTest(
   ctx: Mocha.Context,
   dialect: BuiltInDialect,
-  log?: Logger,
+  overrides?: Omit<KyselyConfig, 'dialect'>,
 ): Promise<TestContext> {
   const config = DB_CONFIGS[dialect]
 
   ctx.timeout(TEST_INIT_TIMEOUT)
-  const db = await connect({
-    ...config,
-    log,
-  })
+  const db = await connect({ ...config, ...overrides })
 
   await createDatabase(db, dialect)
   return { config, db, dialect }

test/node/src/transaction.test.ts

@@ -30,10 +30,12 @@ for (const dialect of DIALECTS) {
     >
 
     before(async function () {
-      ctx = await initTest(this, dialect, (event) => {
-        if (event.level === 'query') {
-          executedQueries.push(event.query)
-        }
+      ctx = await initTest(this, dialect, {
+        log(event) {
+          if (event.level === 'query') {
+            executedQueries.push(event.query)
+          }
+        },
       })
     })