From a2c49f9f2cd46d14c50be3505edaec8d7aff327f Mon Sep 17 00:00:00 2001 From: Rafael Almeida <rafaelalmeidatk@gmail.com> Date: Sun, 2 Feb 2025 15:23:28 -0300 Subject: [PATCH 1/4] Add initial plugin implementation based on Deduplicate Joins --- .../safe-null-comparison-plugin.ts | 28 +++++++++++ .../safe-null-comparison-transformer.ts | 47 +++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts create mode 100644 src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts diff --git a/src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts b/src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts new file mode 100644 index 000000000..e2fe9c8ce --- /dev/null +++ b/src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts @@ -0,0 +1,28 @@ +import { QueryResult } from '../../driver/database-connection.js' +import { RootOperationNode } from '../../query-compiler/query-compiler.js' +import { UnknownRow } from '../../util/type-utils.js' +import { + KyselyPlugin, + PluginTransformQueryArgs, + PluginTransformResultArgs, +} from '../kysely-plugin.js' +import { SafeNullComparisonTransformer } from './safe-null-comparison-transformer.js' + +/** + * Plugin that removes duplicate joins from queries. + * + * See [this recipe](https://github.com/kysely-org/kysely/blob/master/site/docs/recipes/0008-deduplicate-joins.md) + */ +export class SafeNullComparisonPlugin implements KyselyPlugin { + readonly #transformer = new SafeNullComparisonTransformer() + + transformQuery(args: PluginTransformQueryArgs): RootOperationNode { + return this.#transformer.transformNode(args.node) + } + + transformResult( + args: PluginTransformResultArgs, + ): Promise<QueryResult<UnknownRow>> { + return Promise.resolve(args.result) + } +} diff --git a/src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts b/src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts new file mode 100644 index 000000000..27a904dd8 --- /dev/null +++ b/src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts @@ -0,0 +1,47 @@ +import { BinaryOperationNode } from '../../operation-node/binary-operation-node.js' +import { OperationNodeTransformer } from '../../operation-node/operation-node-transformer.js' +import { OperatorNode } from '../../operation-node/operator-node.js' +import { ValueNode } from '../../operation-node/value-node.js' + +export class SafeNullComparisonTransformer extends OperationNodeTransformer { + protected transformBinaryOperation( + node: BinaryOperationNode, + ): BinaryOperationNode { + const { operator, leftOperand, rightOperand } = + super.transformBinaryOperation(node) + + if ( + !ValueNode.is(rightOperand) || + rightOperand.value !== null || + !OperatorNode.is(operator) + ) { + return node + } + + const op = operator.operator + if (op !== '=' && op !== '!=' && op !== '<>') { + return node + } + + return BinaryOperationNode.create( + leftOperand, + OperatorNode.create(op === '=' ? 'is' : 'is not'), + rightOperand, + ) + + // if ( + // OperatorNode.is(transformed.operator) && + // transformed.operator.operator === '=' && + // ValueNode.is(transformed.rightOperand) && + // transformed.rightOperand.value === null + // ) { + // return BinaryOperationNode.create( + // transformed.leftOperand, + // OperatorNode.create('is'), + // ValueNode.createImmediate(null), + // ) + // } + + // return transformed + } +} From 135c6a12e2b9720c764f654f7852a47a9da940c4 Mon Sep 17 00:00:00 2001 From: Rafael Almeida <rafaelalmeidatk@gmail.com> Date: Sun, 2 Feb 2025 16:16:50 -0300 Subject: [PATCH 2/4] Add tests --- src/index.ts | 1 + .../src/safe-null-comparison-plugin.test.ts | 244 ++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 test/node/src/safe-null-comparison-plugin.test.ts diff --git a/src/index.ts b/src/index.ts index 9818f1131..6121f0394 100644 --- a/src/index.ts +++ b/src/index.ts @@ -109,6 +109,7 @@ 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/safe-null-comparison/safe-null-comparison-plugin.js' export * from './operation-node/add-column-node.js' export * from './operation-node/add-constraint-node.js' diff --git a/test/node/src/safe-null-comparison-plugin.test.ts b/test/node/src/safe-null-comparison-plugin.test.ts new file mode 100644 index 000000000..131345d9e --- /dev/null +++ b/test/node/src/safe-null-comparison-plugin.test.ts @@ -0,0 +1,244 @@ +import { SafeNullComparisonPlugin } from '../../..' + +import { + clearDatabase, + destroyTest, + initTest, + TestContext, + testSql, + insertDefaultDataSet, + DIALECTS, +} from './test-setup.js' + +for (const dialect of DIALECTS) { + describe(`${dialect}: safe null comparison`, () => { + let ctx: TestContext + + before(async function () { + ctx = await initTest(this, dialect) + }) + + beforeEach(async () => { + await insertDefaultDataSet(ctx) + }) + + afterEach(async () => { + await clearDatabase(ctx) + }) + + after(async () => { + await destroyTest(ctx) + }) + + it('should replace = with is for null values', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '=', null) + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" is $1', + parameters: [null], + }, + mysql: { + sql: 'select from `person` where `first_name` is ?', + parameters: [null], + }, + mssql: { + sql: 'select from "person" where "first_name" is @1', + parameters: [null], + }, + sqlite: { + sql: 'select from "person" where "first_name" is ?', + parameters: [null], + }, + }) + }) + + it('should not replace = with is for non-null values', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '=', 'Foo') + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" = $1', + parameters: ['Foo'], + }, + mysql: { + sql: 'select from `person` where `first_name` = ?', + parameters: ['Foo'], + }, + mssql: { + sql: 'select from "person" where "first_name" = @1', + parameters: ['Foo'], + }, + sqlite: { + sql: 'select from "person" where "first_name" = ?', + parameters: ['Foo'], + }, + }) + }) + + it('should replace != with is not for null values', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '!=', null) + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" is not $1', + parameters: [null], + }, + mysql: { + sql: 'select from `person` where `first_name` is not ?', + parameters: [null], + }, + mssql: { + sql: 'select from "person" where "first_name" is not @1', + parameters: [null], + }, + sqlite: { + sql: 'select from "person" where "first_name" is not ?', + parameters: [null], + }, + }) + }) + + it('should not replace != with is not for non-null values', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '!=', 'Foo') + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" != $1', + parameters: ['Foo'], + }, + mysql: { + sql: 'select from `person` where `first_name` != ?', + parameters: ['Foo'], + }, + mssql: { + sql: 'select from "person" where "first_name" != @1', + parameters: ['Foo'], + }, + sqlite: { + sql: 'select from "person" where "first_name" != ?', + parameters: ['Foo'], + }, + }) + }) + + it('should replace <> with is not for null values', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '<>', null) + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" is not $1', + parameters: [null], + }, + mysql: { + sql: 'select from `person` where `first_name` is not ?', + parameters: [null], + }, + mssql: { + sql: 'select from "person" where "first_name" is not @1', + parameters: [null], + }, + sqlite: { + sql: 'select from "person" where "first_name" is not ?', + parameters: [null], + }, + }) + }) + + it('should not replace <> with is not for non-null values', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '<>', 'Foo') + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" <> $1', + parameters: ['Foo'], + }, + mysql: { + sql: 'select from `person` where `first_name` <> ?', + parameters: ['Foo'], + }, + mssql: { + sql: 'select from "person" where "first_name" <> @1', + parameters: ['Foo'], + }, + sqlite: { + sql: 'select from "person" where "first_name" <> ?', + parameters: ['Foo'], + }, + }) + }) + + it('should replace = with is with multiple where clauses', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '=', null) + .where('last_name', '=', null) + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" is $1 and "last_name" is $2', + parameters: [null, null], + }, + mysql: { + sql: 'select from `person` where `first_name` is ? and `last_name` is ?', + parameters: [null, null], + }, + mssql: { + sql: 'select from "person" where "first_name" is @1 and "last_name" is @2', + parameters: [null, null], + }, + sqlite: { + sql: 'select from "person" where "first_name" is ? and "last_name" is ?', + parameters: [null, null], + }, + }) + }) + + it('should work with mixed null and non-null values', async () => { + const query = ctx.db + .withPlugin(new SafeNullComparisonPlugin()) + .selectFrom('person') + .where('first_name', '=', null) + .where('last_name', '!=', null) + .where('last_name', '=', 'Foo') + + testSql(query, dialect, { + postgres: { + sql: 'select from "person" where "first_name" is $1 and "last_name" is not $2 and "last_name" = $3', + parameters: [null, null, 'Foo'], + }, + mysql: { + sql: 'select from `person` where `first_name` is ? and `last_name` is not ? and `last_name` = ?', + parameters: [null, null, 'Foo'], + }, + mssql: { + sql: 'select from "person" where "first_name" is @1 and "last_name" is not @2 and "last_name" = @3', + parameters: [null, null, 'Foo'], + }, + sqlite: { + sql: 'select from "person" where "first_name" is ? and "last_name" is not ? and "last_name" = ?', + parameters: [null, null, 'Foo'], + }, + }) + }) + }) +} From 7724c0501c2909e95ccc24683eceb29802c0b429 Mon Sep 17 00:00:00 2001 From: Rafael Almeida <rafaelalmeidatk@gmail.com> Date: Sun, 2 Feb 2025 16:30:48 -0300 Subject: [PATCH 3/4] Add doc comment --- .../safe-null-comparison-plugin.ts | 15 +++++++++++++-- .../safe-null-comparison-transformer.ts | 15 --------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts b/src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts index e2fe9c8ce..80dabf3b7 100644 --- a/src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts +++ b/src/plugin/safe-null-comparison/safe-null-comparison-plugin.ts @@ -9,9 +9,20 @@ import { import { SafeNullComparisonTransformer } from './safe-null-comparison-transformer.js' /** - * Plugin that removes duplicate joins from queries. + * Plugin that handles NULL comparisons to prevent common SQL mistakes. * - * See [this recipe](https://github.com/kysely-org/kysely/blob/master/site/docs/recipes/0008-deduplicate-joins.md) + * In SQL, comparing values with NULL using standard comparison operators (=, !=, <>) + * always yields NULL, which is usually not what developers expect. The correct way + * to compare with NULL is using IS NULL and IS NOT NULL. + * + * When working with nullable variables (e.g. string | null), you need to be careful to + * manually handle these cases with conditional WHERE clauses. This plugins automatically + * applies the correct operator based on the value, allowing you to simply write `query.where('name', '=', name)`. + * + * The plugin transforms the following operators when comparing with NULL: + * - `=` becomes `IS` + * - `!=` becomes `IS NOT` + * - `<>` becomes `IS NOT` */ export class SafeNullComparisonPlugin implements KyselyPlugin { readonly #transformer = new SafeNullComparisonTransformer() diff --git a/src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts b/src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts index 27a904dd8..173d21973 100644 --- a/src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts +++ b/src/plugin/safe-null-comparison/safe-null-comparison-transformer.ts @@ -28,20 +28,5 @@ export class SafeNullComparisonTransformer extends OperationNodeTransformer { OperatorNode.create(op === '=' ? 'is' : 'is not'), rightOperand, ) - - // if ( - // OperatorNode.is(transformed.operator) && - // transformed.operator.operator === '=' && - // ValueNode.is(transformed.rightOperand) && - // transformed.rightOperand.value === null - // ) { - // return BinaryOperationNode.create( - // transformed.leftOperand, - // OperatorNode.create('is'), - // ValueNode.createImmediate(null), - // ) - // } - - // return transformed } } From bebff55a18c06f00b727b88769bf688ae73fd0a2 Mon Sep 17 00:00:00 2001 From: Rafael Almeida <rafaelalmeidatk@gmail.com> Date: Sun, 2 Feb 2025 19:07:23 -0300 Subject: [PATCH 4/4] Add the plugin to the /plugins doc page --- site/docs/plugins.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/site/docs/plugins.md b/site/docs/plugins.md index f9f23230b..c327c8c25 100644 --- a/site/docs/plugins.md +++ b/site/docs/plugins.md @@ -21,3 +21,7 @@ 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). + +### Safe null comparison plugin + +A plugin that automatically converts `=`, `!=` and `<>` to the equivalent `is` and `is not` predicates depending on the value of the variable. [Learn more](https://kysely-org.github.io/kysely-apidoc/classes/SafeNullComparisonPlugin.html).