diff --git a/packages/entity-database-adapter-knex/src/SQLOperator.ts b/packages/entity-database-adapter-knex/src/SQLOperator.ts index a26eb8918..aef0c0f6d 100644 --- a/packages/entity-database-adapter-knex/src/SQLOperator.ts +++ b/packages/entity-database-adapter-knex/src/SQLOperator.ts @@ -48,19 +48,18 @@ export class SQLFragment { * Combine SQL fragments */ append(other: SQLFragment): SQLFragment { - return SQLFragment.join([this, other], ' '); + return joinSQLFragments([this, other], ' '); } /** - * Join multiple SQL fragments into a single fragment with a separator. + * Join multiple SQL fragments with a comma separator. + * Useful for combining column lists, value lists, etc. + * * @param fragments - Array of SQL fragments to join - * @param separator - Separator string (default: ', ') + * @returns - A new SQLFragment with the fragments joined by a comma and space */ - static join(fragments: readonly SQLFragment[], separator = ', '): SQLFragment { - return new SQLFragment( - fragments.map((f) => f.sql).join(separator), - fragments.flatMap((f) => f.bindings), - ); + static joinWithCommaSeparator(...fragments: readonly SQLFragment[]): SQLFragment { + return joinSQLFragments(fragments, ', '); } /** @@ -75,8 +74,8 @@ export class SQLFragment { * // Generates: "SELECT * FROM users WHERE age > ? ORDER BY name" * ``` */ - static concat(...fragments: SQLFragment[]): SQLFragment { - return SQLFragment.join(fragments, ' '); + static concat(...fragments: readonly SQLFragment[]): SQLFragment { + return joinSQLFragments(fragments, ' '); } /** @@ -432,34 +431,40 @@ export const SQLFragmentHelpers = { * JSON path extraction helper (-\>) */ jsonPath(column: string, path: string): SQLFragment { - return new SQLFragment(`"${column}"->'${path}'`, []); + return sql`${identifier(column)}->${path}`; }, /** * JSON path text extraction helper (-\>\>) */ jsonPathText(column: string, path: string): SQLFragment { - return new SQLFragment(`"${column}"->>'${path}'`, []); + return sql`${identifier(column)}->>${path}`; }, /** * Logical AND of multiple fragments */ - and(...conditions: SQLFragment[]): SQLFragment { + and(...conditions: readonly SQLFragment[]): SQLFragment { if (conditions.length === 0) { return sql`1 = 1`; } - return SQLFragment.join(conditions, ' AND '); + return joinSQLFragments( + conditions.map((c) => SQLFragmentHelpers.group(c)), + ' AND ', + ); }, /** * Logical OR of multiple fragments */ - or(...conditions: SQLFragment[]): SQLFragment { + or(...conditions: readonly SQLFragment[]): SQLFragment { if (conditions.length === 0) { return sql`1 = 0`; } - return SQLFragment.join(conditions, ' OR '); + return joinSQLFragments( + conditions.map((c) => SQLFragmentHelpers.group(c)), + ' OR ', + ); }, /** @@ -476,3 +481,11 @@ export const SQLFragmentHelpers = { return new SQLFragment('(' + condition.sql + ')', condition.bindings); }, }; + +// Internal helper function to join SQL fragments with a specified separator +function joinSQLFragments(fragments: readonly SQLFragment[], separator: string): SQLFragment { + return new SQLFragment( + fragments.map((f) => f.sql).join(separator), + fragments.flatMap((f) => f.bindings), + ); +} diff --git a/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts b/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts index 92f95e4ab..59294c699 100644 --- a/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts +++ b/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts @@ -13,7 +13,7 @@ import { setTimeout } from 'timers/promises'; import { PaginationSpecification } from '../AuthorizationResultBasedKnexEntityLoader'; import { NullsOrdering, OrderByOrdering } from '../BasePostgresEntityDatabaseAdapter'; import { PaginationStrategy } from '../PaginationStrategy'; -import { raw, sql, SQLFragment, SQLFragmentHelpers } from '../SQLOperator'; +import { raw, sql, SQLFragmentHelpers } from '../SQLOperator'; import { PostgresTestEntity, PostgresTestEntityFields, @@ -695,7 +695,7 @@ describe('postgres entity integration', () => { sql`(has_a_cat = ${true} AND has_a_dog = ${true})`, ]; const joinedResults = await PostgresTestEntity.knexLoader(vc1) - .loadManyBySQL(SQLFragment.join(conditions, ' OR ')) + .loadManyBySQL(SQLFragmentHelpers.or(...conditions)) .orderBy('name', OrderByOrdering.ASCENDING) .executeAsync(); diff --git a/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts b/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts index 410e5d31c..952b8d66f 100644 --- a/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts +++ b/packages/entity-database-adapter-knex/src/__tests__/SQLOperator-test.ts @@ -109,45 +109,25 @@ describe('SQLOperator', () => { }); }); - describe(SQLFragment.join, () => { - it('joins fragments with custom separator', () => { - const fragments = [ - new SQLFragment('name = ?', [{ type: 'value', value: 'Alice' }]), - new SQLFragment('age = ?', [{ type: 'value', value: 30 }]), - new SQLFragment('city = ?', [{ type: 'value', value: 'NYC' }]), - ]; - const joined = SQLFragment.join(fragments, ' AND '); - - expect(joined.sql).toBe('name = ? AND age = ? AND city = ?'); - expect(joined.getKnexBindings()).toEqual(['Alice', 30, 'NYC']); - }); - + describe(SQLFragment.joinWithCommaSeparator, () => { it('handles empty array in join', () => { - const joined = SQLFragment.join([]); + const joined = SQLFragment.joinWithCommaSeparator(); expect(joined.sql).toBe(''); expect(joined.getKnexBindings()).toEqual([]); }); - it('joins SQL fragments with default separator', () => { + it('joins SQL fragments with comma', () => { const columns = [sql`name`, sql`age`, sql`email`]; - const joined = SQLFragment.join(columns); + const joined = SQLFragment.joinWithCommaSeparator(...columns); expect(joined.sql).toBe('name, age, email'); expect(joined.getKnexBindings()).toEqual([]); }); - it('joins SQL fragments with custom separator', () => { - const conditions = [sql`age > ${18}`, sql`status = ${'active'}`, sql`verified = ${true}`]; - const joined = SQLFragment.join(conditions, ' AND '); - - expect(joined.sql).toBe('age > ? AND status = ? AND verified = ?'); - expect(joined.getKnexBindings()).toEqual([18, 'active', true]); - }); - it('handles single fragment', () => { const single = [sql`name = ${'Alice'}`]; - const joined = SQLFragment.join(single); + const joined = SQLFragment.joinWithCommaSeparator(...single); expect(joined.sql).toBe('name = ?'); expect(joined.getKnexBindings()).toEqual(['Alice']); @@ -191,7 +171,7 @@ describe('SQLOperator', () => { filters.push(sql`category = ${'electronics'}`); if (filters.length > 0) { - fragments.push(sql`WHERE ${SQLFragment.join(filters, ' AND ')}`); + fragments.push(sql`WHERE ${SQLFragmentHelpers.and(...filters)}`); } // Add ORDER BY @@ -203,7 +183,7 @@ describe('SQLOperator', () => { const query = SQLFragment.concat(...fragments); expect(query.sql).toBe( - 'SELECT * FROM products WHERE price > ? AND category = ? ORDER BY created_at DESC LIMIT ?', + 'SELECT * FROM products WHERE (price > ?) AND (category = ?) ORDER BY created_at DESC LIMIT ?', ); expect(query.getKnexBindings()).toEqual([100, 'electronics', 10]); }); @@ -632,8 +612,8 @@ describe('SQLOperator', () => { it('generates JSON path access', () => { const fragment = SQLFragmentHelpers.jsonPath('data', 'user'); - expect(fragment.sql).toBe(`"data"->'user'`); - expect(fragment.getKnexBindings()).toEqual([]); + expect(fragment.sql).toBe(`??->?`); + expect(fragment.getKnexBindings()).toEqual(['data', 'user']); }); }); @@ -641,8 +621,8 @@ describe('SQLOperator', () => { it('generates JSON path text access', () => { const fragment = SQLFragmentHelpers.jsonPathText('data', 'email'); - expect(fragment.sql).toBe(`"data"->>'email'`); - expect(fragment.getKnexBindings()).toEqual([]); + expect(fragment.sql).toBe(`??->>?`); + expect(fragment.getKnexBindings()).toEqual(['data', 'email']); }); }); @@ -652,7 +632,7 @@ describe('SQLOperator', () => { const cond2 = sql`status = ${'active'}`; const fragment = SQLFragmentHelpers.and(cond1, cond2); - expect(fragment.sql).toBe('age >= ? AND status = ?'); + expect(fragment.sql).toBe('(age >= ?) AND (status = ?)'); expect(fragment.getKnexBindings()).toEqual([18, 'active']); }); @@ -660,7 +640,7 @@ describe('SQLOperator', () => { const cond = sql`age >= ${18}`; const fragment = SQLFragmentHelpers.and(cond); - expect(fragment.sql).toBe('age >= ?'); + expect(fragment.sql).toBe('(age >= ?)'); expect(fragment.getKnexBindings()).toEqual([18]); }); @@ -678,7 +658,7 @@ describe('SQLOperator', () => { const cond2 = sql`status = ${'pending'}`; const fragment = SQLFragmentHelpers.or(cond1, cond2); - expect(fragment.sql).toBe('status = ? OR status = ?'); + expect(fragment.sql).toBe('(status = ?) OR (status = ?)'); expect(fragment.getKnexBindings()).toEqual(['active', 'pending']); }); @@ -686,7 +666,7 @@ describe('SQLOperator', () => { const cond = sql`status = ${'active'}`; const fragment = SQLFragmentHelpers.or(cond); - expect(fragment.sql).toBe('status = ?'); + expect(fragment.sql).toBe('(status = ?)'); expect(fragment.getKnexBindings()).toEqual(['active']); }); @@ -729,7 +709,7 @@ describe('SQLOperator', () => { ); expect(fragment.sql).toBe( - '?? BETWEEN ? AND ? AND (?? IN (?, ?) OR role = ?) AND ?? IS NOT NULL', + '(?? BETWEEN ? AND ?) AND (((?? IN (?, ?)) OR (role = ?))) AND (?? IS NOT NULL)', ); expect(fragment.getKnexBindings()).toEqual([ 'age', diff --git a/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts b/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts index 9268c77aa..6e28d0318 100644 --- a/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts +++ b/packages/entity-database-adapter-knex/src/internal/EntityKnexDataManager.ts @@ -594,7 +594,7 @@ export class EntityKnexDataManager< ); // Build left side of comparison (current row's computed values) - const leftSide = SQLFragment.join(postgresCursorFieldIdentifiers, ', '); + const leftSide = SQLFragment.joinWithCommaSeparator(...postgresCursorFieldIdentifiers); // Build right side using subquery to get computed values for cursor entity. // For field names, qualify with the cursor row alias. For SQL fragments, @@ -605,7 +605,7 @@ export class EntityKnexDataManager< // Build SELECT fields for subquery const rightSideSubquery = sql` - SELECT ${SQLFragment.join(postgresCursorRowFieldIdentifiers, ', ')} + SELECT ${SQLFragment.joinWithCommaSeparator(...postgresCursorRowFieldIdentifiers)} FROM ${identifier(tableName)} AS ${raw(CURSOR_ROW_TABLE_ALIAS)} WHERE ${raw(CURSOR_ROW_TABLE_ALIAS)}.${identifier(idField)} = ${decodedExternalCursorEntityID} `; @@ -637,7 +637,7 @@ export class EntityKnexDataManager< tableAlias?: typeof CURSOR_ROW_TABLE_ALIAS, ): SQLFragment { const ilikeConditions = this.buildILikeConditions(search, tableAlias); - return sql`CASE WHEN ${SQLFragment.join(ilikeConditions, ' OR ')} THEN 1 ELSE 0 END`; + return sql`CASE WHEN ${SQLFragmentHelpers.or(...ilikeConditions)} THEN 1 ELSE 0 END`; } private buildTrigramSimilarityGreatestExpression( @@ -645,7 +645,7 @@ export class EntityKnexDataManager< tableAlias?: typeof CURSOR_ROW_TABLE_ALIAS, ): SQLFragment { const similarityExprs = this.buildTrigramSimilarityExpressions(search, tableAlias); - return sql`GREATEST(${SQLFragment.join(similarityExprs, ', ')})`; + return sql`GREATEST(${SQLFragment.joinWithCommaSeparator(...similarityExprs)})`; } private buildTrigramCursorCondition( @@ -671,9 +671,11 @@ export class EntityKnexDataManager< extraOrderByFields?.map((f) => this.resolveSearchFieldToSQLFragment(f)) ?? []; // Build left side of comparison (current row's computed values) - const leftSide = SQLFragment.join( - [exactMatchExpr, similarityExpr, ...extraFields, sql`${identifier(idField)}`], - ', ', + const leftSide = SQLFragment.joinWithCommaSeparator( + exactMatchExpr, + similarityExpr, + ...extraFields, + sql`${identifier(idField)}`, ); // Build right side using subquery to get computed values for cursor entity @@ -702,7 +704,7 @@ export class EntityKnexDataManager< ]; const rightSideSubquery = sql` - SELECT ${SQLFragment.join(selectFields, ', ')} + SELECT ${SQLFragment.joinWithCommaSeparator(...selectFields)} FROM ${identifier(this.entityConfiguration.tableName)} AS ${raw(CURSOR_ROW_TABLE_ALIAS)} WHERE ${raw(CURSOR_ROW_TABLE_ALIAS)}.${identifier(idField)} = ${decodedExternalCursorEntityID} `; @@ -733,7 +735,7 @@ export class EntityKnexDataManager< ]; return { - searchWhere: conditions.length > 0 ? SQLFragment.join(conditions, ' OR ') : sql`1 = 0`, + searchWhere: conditions.length > 0 ? SQLFragmentHelpers.or(...conditions) : sql`1 = 0`, searchOrderByClauses, }; } @@ -777,7 +779,7 @@ export class EntityKnexDataManager< ]; return { - searchWhere: SQLFragment.join(allConditions, ' OR '), + searchWhere: SQLFragmentHelpers.or(...allConditions), searchOrderByClauses, }; }