From e2c6d77df2babfffccff02fc92b7b1f126fe3b64 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Sun, 25 Oct 2026 04:15:46 +0000 Subject: [PATCH] fix(deparser): support ALTER STATISTICS RENAME and quote/validate operator names --- __fixtures__/generated/generated.json | 27 ++++ .../kitchen-sink/misc/alter-statistics.sql | 9 ++ .../kitchen-sink/misc/operator-names.sql | 18 +++ .../misc-alter-statistics.test.ts | 17 +++ .../kitchen-sink/misc-operator-names.test.ts | 26 ++++ .../__tests__/misc/operator-names.test.ts | 38 ++++++ packages/deparser/src/deparser.ts | 125 ++++++++---------- 7 files changed, 191 insertions(+), 69 deletions(-) create mode 100644 __fixtures__/kitchen-sink/misc/alter-statistics.sql create mode 100644 __fixtures__/kitchen-sink/misc/operator-names.sql create mode 100644 packages/deparser/__tests__/kitchen-sink/misc-alter-statistics.test.ts create mode 100644 packages/deparser/__tests__/kitchen-sink/misc-operator-names.test.ts create mode 100644 packages/deparser/__tests__/misc/operator-names.test.ts diff --git a/__fixtures__/generated/generated.json b/__fixtures__/generated/generated.json index a9ad7570..2e26bffe 100644 --- a/__fixtures__/generated/generated.json +++ b/__fixtures__/generated/generated.json @@ -21526,6 +21526,24 @@ "misc/pg18-copy-18.sql": "COPY t FROM 'it''s.csv'", "misc/pg18-copy-19.sql": "COPY t FROM STDIN WHERE a > 0", "misc/pg18-copy-20.sql": "COPY t FROM STDIN WITH NULL AS 'I''m null'", + "misc/operator-names-1.sql": "SELECT a OPERATOR(pg_catalog.+) b FROM t", + "misc/operator-names-2.sql": "SELECT a OPERATOR(\"My Schema\".+) b FROM t", + "misc/operator-names-3.sql": "SELECT a OPERATOR(\"S\".===) b OPERATOR(public.*) c FROM t", + "misc/operator-names-4.sql": "SELECT OPERATOR(\"S\".-) a FROM t", + "misc/operator-names-5.sql": "SELECT a OPERATOR(\"S\".=) ANY (ARRAY[1]) FROM t", + "misc/operator-names-6.sql": "SELECT a OPERATOR(\"S\".=) ALL (SELECT 1) FROM t", + "misc/operator-names-7.sql": "SELECT - -1, @ -1, 1 - -1, 2 ^ - 1", + "misc/operator-names-8.sql": "CREATE OPERATOR \"My Schema\".=== (LEFTARG = int, RIGHTARG = int, PROCEDURE = f, COMMUTATOR = OPERATOR(\"My Schema\".===), NEGATOR = OPERATOR(\"My Schema\".!==))", + "misc/operator-names-9.sql": "CREATE OPERATOR === (LEFTARG = int, RIGHTARG = int, PROCEDURE = f, COMMUTATOR = ===, NEGATOR = !==)", + "misc/operator-names-10.sql": "ALTER OPERATOR === (int, int) SET (COMMUTATOR = OPERATOR(\"My Schema\".===))", + "misc/operator-names-11.sql": "ALTER OPERATOR \"S\".=== (int, int) SET (NEGATOR = !==)", + "misc/operator-names-12.sql": "DROP OPERATOR \"S\".< (int, int)", + "misc/operator-names-13.sql": "COMMENT ON OPERATOR \"My Schema\".=== (int, int) IS 'x'", + "misc/operator-names-14.sql": "COMMENT ON OPERATOR - (NONE, int) IS 'x'", + "misc/operator-names-15.sql": "CREATE OPERATOR CLASS c FOR TYPE int USING btree AS OPERATOR 1 \"S\".< (int, int), OPERATOR 2 <=, FUNCTION 1 f(int, int)", + "misc/operator-names-16.sql": "ALTER OPERATOR FAMILY f USING btree ADD OPERATOR 1 \"S\".< (int, int)", + "misc/operator-names-17.sql": "ALTER TABLE t ADD EXCLUDE USING gist (c WITH OPERATOR(\"S\".&&))", + "misc/operator-names-18.sql": "ALTER TABLE t ADD EXCLUDE USING gist (c WITH &&)", "misc/missing-types-1.sql": "ALTER PUBLICATION \"my_publication\" OWNER TO \"postgres\"", "misc/missing-types-2.sql": "ALTER OPERATOR FAMILY my_opfamily USING btree OWNER TO new_owner", "misc/missing-types-3.sql": "ALTER OPERATOR CLASS my_opclass USING btree OWNER TO new_owner", @@ -21765,6 +21783,15 @@ "misc/booleans-cast-1.sql": "SELECT * FROM myschema.mytable WHERE a = TRUE", "misc/booleans-cast-2.sql": "SELECT * FROM myschema.mytable WHERE a = CAST('t' AS boolean)", "misc/booleans-cast-3.sql": "SELECT * FROM myschema.mytable WHERE a = 't'::boolean", + "misc/alter-statistics-1.sql": "ALTER STATISTICS ab1_a_b_stats RENAME TO ab1_a_b_stats_new", + "misc/alter-statistics-2.sql": "ALTER STATISTICS \"My Schema\".\"My Stats\" RENAME TO \"New Stats\"", + "misc/alter-statistics-3.sql": "ALTER STATISTICS public.s1 RENAME TO s2", + "misc/alter-statistics-4.sql": "ALTER STATISTICS s1 SET SCHEMA s2", + "misc/alter-statistics-5.sql": "ALTER STATISTICS s1 OWNER TO r1", + "misc/alter-statistics-6.sql": "ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0", + "misc/alter-statistics-7.sql": "ALTER STATISTICS IF EXISTS ab1_a_b_stats_new SET STATISTICS -1", + "misc/alter-statistics-8.sql": "ALTER STATISTICS s1 SET STATISTICS DEFAULT", + "misc/alter-statistics-9.sql": "ALTER STATISTICS \"My Schema\".\"My Stats\" SET STATISTICS 5", "latest/postgres/create_view-1.sql": "CREATE FUNCTION interpt_pp(path, path)\n RETURNS point\n AS 'regresslib'\n LANGUAGE C STRICT", "latest/postgres/create_view-2.sql": "CREATE TABLE real_city (\n\tpop\t\t\tint4,\n\tcname\t\ttext,\n\toutline \tpath\n)", "latest/postgres/create_view-3.sql": "COPY real_city FROM 'filename'", diff --git a/__fixtures__/kitchen-sink/misc/alter-statistics.sql b/__fixtures__/kitchen-sink/misc/alter-statistics.sql new file mode 100644 index 00000000..a40ebbbb --- /dev/null +++ b/__fixtures__/kitchen-sink/misc/alter-statistics.sql @@ -0,0 +1,9 @@ +ALTER STATISTICS ab1_a_b_stats RENAME TO ab1_a_b_stats_new; +ALTER STATISTICS "My Schema"."My Stats" RENAME TO "New Stats"; +ALTER STATISTICS public.s1 RENAME TO s2; +ALTER STATISTICS s1 SET SCHEMA s2; +ALTER STATISTICS s1 OWNER TO r1; +ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0; +ALTER STATISTICS IF EXISTS ab1_a_b_stats_new SET STATISTICS -1; +ALTER STATISTICS s1 SET STATISTICS DEFAULT; +ALTER STATISTICS "My Schema"."My Stats" SET STATISTICS 5; diff --git a/__fixtures__/kitchen-sink/misc/operator-names.sql b/__fixtures__/kitchen-sink/misc/operator-names.sql new file mode 100644 index 00000000..374cb39b --- /dev/null +++ b/__fixtures__/kitchen-sink/misc/operator-names.sql @@ -0,0 +1,18 @@ +SELECT a OPERATOR(pg_catalog.+) b FROM t; +SELECT a OPERATOR("My Schema".+) b FROM t; +SELECT a OPERATOR("S".===) b OPERATOR(public.*) c FROM t; +SELECT OPERATOR("S".-) a FROM t; +SELECT a OPERATOR("S".=) ANY (ARRAY[1]) FROM t; +SELECT a OPERATOR("S".=) ALL (SELECT 1) FROM t; +SELECT - -1, @ -1, 1 - -1, 2 ^ - 1; +CREATE OPERATOR "My Schema".=== (LEFTARG = int, RIGHTARG = int, PROCEDURE = f, COMMUTATOR = OPERATOR("My Schema".===), NEGATOR = OPERATOR("My Schema".!==)); +CREATE OPERATOR === (LEFTARG = int, RIGHTARG = int, PROCEDURE = f, COMMUTATOR = ===, NEGATOR = !==); +ALTER OPERATOR === (int, int) SET (COMMUTATOR = OPERATOR("My Schema".===)); +ALTER OPERATOR "S".=== (int, int) SET (NEGATOR = !==); +DROP OPERATOR "S".< (int, int); +COMMENT ON OPERATOR "My Schema".=== (int, int) IS 'x'; +COMMENT ON OPERATOR - (NONE, int) IS 'x'; +CREATE OPERATOR CLASS c FOR TYPE int USING btree AS OPERATOR 1 "S".< (int, int), OPERATOR 2 <=, FUNCTION 1 f(int, int); +ALTER OPERATOR FAMILY f USING btree ADD OPERATOR 1 "S".< (int, int); +ALTER TABLE t ADD EXCLUDE USING gist (c WITH OPERATOR("S".&&)); +ALTER TABLE t ADD EXCLUDE USING gist (c WITH &&); diff --git a/packages/deparser/__tests__/kitchen-sink/misc-alter-statistics.test.ts b/packages/deparser/__tests__/kitchen-sink/misc-alter-statistics.test.ts new file mode 100644 index 00000000..14c4da27 --- /dev/null +++ b/packages/deparser/__tests__/kitchen-sink/misc-alter-statistics.test.ts @@ -0,0 +1,17 @@ + +import { FixtureTestUtils } from '../../test-utils'; +const fixtures = new FixtureTestUtils(); + +it('misc-alter-statistics', async () => { + await fixtures.runFixtureTests([ + "misc/alter-statistics-1.sql", + "misc/alter-statistics-2.sql", + "misc/alter-statistics-3.sql", + "misc/alter-statistics-4.sql", + "misc/alter-statistics-5.sql", + "misc/alter-statistics-6.sql", + "misc/alter-statistics-7.sql", + "misc/alter-statistics-8.sql", + "misc/alter-statistics-9.sql" +]); +}); diff --git a/packages/deparser/__tests__/kitchen-sink/misc-operator-names.test.ts b/packages/deparser/__tests__/kitchen-sink/misc-operator-names.test.ts new file mode 100644 index 00000000..c942c4f3 --- /dev/null +++ b/packages/deparser/__tests__/kitchen-sink/misc-operator-names.test.ts @@ -0,0 +1,26 @@ + +import { FixtureTestUtils } from '../../test-utils'; +const fixtures = new FixtureTestUtils(); + +it('misc-operator-names', async () => { + await fixtures.runFixtureTests([ + "misc/operator-names-1.sql", + "misc/operator-names-2.sql", + "misc/operator-names-3.sql", + "misc/operator-names-4.sql", + "misc/operator-names-5.sql", + "misc/operator-names-6.sql", + "misc/operator-names-7.sql", + "misc/operator-names-8.sql", + "misc/operator-names-9.sql", + "misc/operator-names-10.sql", + "misc/operator-names-11.sql", + "misc/operator-names-12.sql", + "misc/operator-names-13.sql", + "misc/operator-names-14.sql", + "misc/operator-names-15.sql", + "misc/operator-names-16.sql", + "misc/operator-names-17.sql", + "misc/operator-names-18.sql" +]); +}); diff --git a/packages/deparser/__tests__/misc/operator-names.test.ts b/packages/deparser/__tests__/misc/operator-names.test.ts new file mode 100644 index 00000000..ff42419b --- /dev/null +++ b/packages/deparser/__tests__/misc/operator-names.test.ts @@ -0,0 +1,38 @@ +import { Deparser } from '../../src'; + +// The parser can only produce operator names that lex back as one operator token, +// so names that would not round-trip have to be hand-built. +const opExpr = (...name: string[]) => ({ + A_Expr: { + kind: 'AEXPR_OP', + name: name.map(sval => ({ String: { sval } })), + lexpr: { ColumnRef: { fields: [{ String: { sval: 'a' } }] } }, + rexpr: { ColumnRef: { fields: [{ String: { sval: 'b' } }] } } + } +}); + +describe('operator names', () => { + it.each([ + ['+-'], + ['*-'], + ['--'], + ['/*'], + ['!='], + ['=>'], + ['and'], + [''], + ['<'.repeat(64)] + ])('rejects %j, which would not reparse as the same operator', op => { + expect(() => Deparser.deparse(opExpr(op) as any)).toThrow(/Invalid operator name/); + expect(() => Deparser.deparse(opExpr('s', op) as any)).toThrow(/Invalid operator name/); + }); + + it.each([ + [['+'], 'a + b'], + [['@-@'], 'a @-@ b'], + [['~-'], 'a ~- b'], + [['My Schema', '<->'], 'a OPERATOR("My Schema".<->) b'] + ])('keeps %j', (name, expected) => { + expect(Deparser.deparse(opExpr(...name) as any)).toBe(expected); + }); +}); diff --git a/packages/deparser/src/deparser.ts b/packages/deparser/src/deparser.ts index 94c7f625..ee803fbf 100644 --- a/packages/deparser/src/deparser.ts +++ b/packages/deparser/src/deparser.ts @@ -965,18 +965,36 @@ export class Deparser implements DeparserVisitor { return ''; } - const parts = name.map((n: any) => { - if (n.String) { - return n.String.sval || n.String.str; - } - return this.visit(n, context); - }); + const qualified = this.qualifiedOperatorName(name); + return name.length > 1 ? `OPERATOR(${qualified})` : qualified; + } - if (parts.length > 1) { - return `OPERATOR(${parts.join('.')})`; + /** + * Renders `schema.op` with the schema quoted. Throws if the operator symbol + * would not lex back as a single operator token (e.g. `+-`, `--`, `!=`). + */ + qualifiedOperatorName(name: t.Node[]): string { + const parts = ListUtils.unwrapList(name).map((n: any) => n.String ? (n.String.sval ?? n.String.str ?? '') : ''); + const op = parts.pop() ?? ''; + if (!this.isValidOperatorSymbol(op)) { + throw new Error(`Invalid operator name: ${JSON.stringify(op)}`); } + return [...parts.map(part => QuoteUtils.quoteIdentifier(part)), op].join('.'); + } + + private isValidOperatorSymbol(op: string): boolean { + if (!/^[~!@#^&|`?+\-*/%<>=]+$/.test(op) || op.length >= 64) return false; + if (op.includes('--') || op.includes('/*')) return false; + // The lexer rewrites `!=` to `<>` and treats `=>` as its own token. + if (op === '!=' || op === '=>') return false; + // A multi-char operator may only end in + or - if it contains one of these. + return op.length === 1 || !/[+-]$/.test(op) || /[~!@#^&|`?%]/.test(op); + } - return parts.join('.'); + private isOperatorNameList(arg: t.Node | undefined): boolean { + if (!arg || this.getNodeType(arg) !== 'List') return false; + const items = ListUtils.unwrapList(this.getNodeData(arg).items); + return items.length > 0 && items.every((item: any) => item.String); } private getOperatorPrecedence(operator: string): number { @@ -3211,10 +3229,7 @@ export class Deparser implements DeparserVisitor { const operatorNode = elemList[1]; let operator = ''; if (this.getNodeType(operatorNode) === 'List') { - const operatorList = ListUtils.unwrapList(operatorNode); - if (operatorList.length > 0 && operatorList[0].String) { - operator = operatorList[0].String.sval; - } + operator = this.deparseOperatorName(ListUtils.unwrapList(operatorNode), context); } else if (operatorNode.String) { operator = operatorNode.String.sval; } else { @@ -5941,13 +5956,9 @@ export class Deparser implements DeparserVisitor { // Handle CREATE OPERATOR commutator/negator - MUST be first to prevent quoting if (context.parentNodeTypes.includes('DefineStmt') && ['commutator', 'negator'].includes(node.defname.toLowerCase())) { - if (node.arg && this.getNodeType(node.arg) === 'List') { - const listData = this.getNodeData(node.arg); - const listItems = ListUtils.unwrapList(listData.items); - if (listItems.length === 1 && listItems[0].String) { - const preservedName = this.preserveOperatorDefElemCase(node.defname); - return `${preservedName} = ${listItems[0].String.sval}`; - } + if (this.isOperatorNameList(node.arg)) { + const preservedName = this.preserveOperatorDefElemCase(node.defname); + return `${preservedName} = ${this.deparseOperatorName(ListUtils.unwrapList(this.getNodeData(node.arg).items), context)}`; } } @@ -6053,12 +6064,8 @@ export class Deparser implements DeparserVisitor { } } - if (node.arg && this.getNodeType(node.arg) === 'List') { - const listData = this.getNodeData(node.arg); - const listItems = ListUtils.unwrapList(listData.items); - if (listItems.length === 1 && listItems[0].String) { - return `${node.defname} = ${listItems[0].String.sval}`; - } + if (this.isOperatorNameList(node.arg)) { + return `${node.defname} = ${this.deparseOperatorName(ListUtils.unwrapList(this.getNodeData(node.arg).items), context)}`; } } @@ -6423,12 +6430,8 @@ export class Deparser implements DeparserVisitor { if (['commutator', 'negator'].includes(node.defname.toLowerCase())) { if (node.arg) { // For commutator/negator, preserve the original operator symbol without quotes - if (node.arg && this.getNodeType(node.arg) === 'List') { - const listData = this.getNodeData(node.arg); - const listItems = ListUtils.unwrapList(listData.items); - if (listItems.length === 1 && listItems[0].String) { - return `${preservedName} = ${listItems[0].String.sval}`; - } + if (this.isOperatorNameList(node.arg)) { + return `${preservedName} = ${this.deparseOperatorName(ListUtils.unwrapList(this.getNodeData(node.arg).items), context)}`; } const unquotedValue = argValue.replace(/^"(.*)"$/, '$1'); return `${preservedName} = ${unquotedValue}`; @@ -7120,12 +7123,7 @@ export class Deparser implements DeparserVisitor { // For operators, we need to handle ObjectWithArgs structure if (node.object && (node.object as any).ObjectWithArgs) { const objWithArgs = (node.object as any).ObjectWithArgs; - let operatorName = objWithArgs.objname && objWithArgs.objname[0] && objWithArgs.objname[0].String - ? objWithArgs.objname[0].String.sval : 'unknown'; - - if (operatorName.startsWith('"') && operatorName.endsWith('"')) { - operatorName = operatorName.slice(1, -1); - } + const operatorName = this.qualifiedOperatorName(objWithArgs.objname); const args: string[] = []; if (objWithArgs.objargs) { @@ -7182,13 +7180,7 @@ export class Deparser implements DeparserVisitor { } else if (node.objtype === 'OBJECT_OPERATOR' && node.object && (node.object as any).ObjectWithArgs) { // Handle direct ObjectWithArgs for OPERATOR syntax: COMMENT ON OPERATOR -(NONE, integer) IS 'comment' const objWithArgs = (node.object as any).ObjectWithArgs; - let operatorName = objWithArgs.objname && objWithArgs.objname[0] && objWithArgs.objname[0].String - ? objWithArgs.objname[0].String.sval : 'unknown'; - - // Remove quotes from operator name if present - if (operatorName.startsWith('"') && operatorName.endsWith('"')) { - operatorName = operatorName.slice(1, -1); - } + const operatorName = this.qualifiedOperatorName(objWithArgs.objname); const args: string[] = []; if (objWithArgs.objargs) { @@ -7958,24 +7950,28 @@ export class Deparser implements DeparserVisitor { AlterStatsStmt(node: t.AlterStatsStmt, context: DeparserContext): string { const output: string[] = ['ALTER', 'STATISTICS']; + if (node.missing_ok) { + output.push('IF EXISTS'); + } + if (node.defnames && node.defnames.length > 0) { - const names = ListUtils.unwrapList(node.defnames).map(name => this.visit(name, context)); + const names = ListUtils.unwrapList(node.defnames).map(name => QuoteUtils.quoteIdentifier(this.getNodeData(name).sval)); output.push(names.join('.')); } output.push('SET', 'STATISTICS'); - if (node.stxstattarget) { - output.push(this.visit(node.stxstattarget, context)); - } + output.push(node.stxstattarget ? this.visit(node.stxstattarget, context) : 'DEFAULT'); return output.join(' '); } - ObjectWithArgs(node: t.ObjectWithArgs, context: DeparserContext): string { + ObjectWithArgs(node: t.ObjectWithArgs, context: DeparserContext, isOperator = context.objtype === 'OBJECT_OPERATOR'): string { let result = ''; - if (node.objname && node.objname.length > 0) { + if (isOperator && node.objname && node.objname.length > 0) { + result = this.qualifiedOperatorName(node.objname); + } else if (node.objname && node.objname.length > 0) { const objContext = context.spawn('ObjectWithArgs'); const names = ListUtils.unwrapList(node.objname).map(name => this.visit(name, objContext)); result = names.join('.'); @@ -8013,7 +8009,7 @@ export class Deparser implements DeparserVisitor { const output: string[] = ['ALTER', 'OPERATOR']; if (node.opername) { - output.push(this.ObjectWithArgs(node.opername, context)); + output.push(this.ObjectWithArgs(node.opername, context, true)); } output.push('SET'); @@ -8514,6 +8510,9 @@ export class Deparser implements DeparserVisitor { case 'OBJECT_ROUTINE': output.push('ROUTINE'); break; + case 'OBJECT_STATISTIC_EXT': + output.push('STATISTICS'); + break; default: throw new Error(`Unsupported RenameStmt renameType: ${node.renameType}`); } @@ -8547,7 +8546,7 @@ export class Deparser implements DeparserVisitor { } else { output.push(this.visit(node.object, context)); } - } else if ((node.renameType === 'OBJECT_TYPE' || node.renameType === 'OBJECT_DOMAIN') && (node.object as any).List) { + } else if ((node.renameType === 'OBJECT_TYPE' || node.renameType === 'OBJECT_DOMAIN' || node.renameType === 'OBJECT_STATISTIC_EXT') && (node.object as any).List) { // Qualified type names - join List parts with dots const items = ListUtils.unwrapList(node.object as any); const parts = items @@ -10094,16 +10093,7 @@ export class Deparser implements DeparserVisitor { output.push('CREATE OPERATOR'); if (node.defnames && node.defnames.length > 0) { - const names = ListUtils.unwrapList(node.defnames).map((name, index) => { - if (index === node.defnames.length - 1) { - const nodeData = this.getNodeData(name); - if (nodeData && nodeData.sval) { - return nodeData.sval; // Return operator symbol unquoted - } - } - return this.visit(name, context); // Quote schema/namespace names normally - }); - output.push(names.join('.')); + output.push(this.qualifiedOperatorName(node.defnames)); } if (node.definition && node.definition.length > 0) { @@ -10118,11 +10108,8 @@ export class Deparser implements DeparserVisitor { const quotedDefName = QuoteUtils.quoteIdentifier(defName); const preservedDefName = quotedDefName !== defName ? quotedDefName : this.preserveOperatorDefElemCase(defName); - if ((defName.toLowerCase() === 'commutator' || defName.toLowerCase() === 'negator') && defValue.List) { - const listItems = ListUtils.unwrapList(defValue.List.items); - if (listItems.length === 1 && listItems[0].String) { - return `${preservedDefName} = ${listItems[0].String.sval}`; - } + if ((defName.toLowerCase() === 'commutator' || defName.toLowerCase() === 'negator') && this.isOperatorNameList(defValue)) { + return `${preservedDefName} = ${this.deparseOperatorName(ListUtils.unwrapList(defValue.List.items), context)}`; } // For commutator/negator, we already handled them above if ((defName.toLowerCase() === 'commutator' || defName.toLowerCase() === 'negator')) { @@ -10706,7 +10693,7 @@ export class Deparser implements DeparserVisitor { output.push(operatorNumber.toString()); if (node.name) { const opClassContext = context.spawn('CreateOpClassItem'); - output.push(this.ObjectWithArgs(node.name, opClassContext)); + output.push(this.ObjectWithArgs(node.name, opClassContext, true)); } } else if (node.itemtype === 2) { output.push('FUNCTION');