Skip to content

fix(pretty-print): quote reserved-keyword and mixed-case identifiers across all emitters#755

Merged
psteinroe merged 2 commits into
supabase-community:mainfrom
JohannesKnopp:fix/index-elem-reserved-keyword-quoting
Jun 3, 2026
Merged

fix(pretty-print): quote reserved-keyword and mixed-case identifiers across all emitters#755
psteinroe merged 2 commits into
supabase-community:mainfrom
JohannesKnopp:fix/index-elem-reserved-keyword-quoting

Conversation

@JohannesKnopp
Copy link
Copy Markdown
Contributor

Several emitters printed identifiers as bare tokens instead of using the quote-aware helper, so names that require quoting (reserved keywords, multi-word, mixed-case) lost their qutes and produced invalid or semantically different SQL.

Similar to (#749), routed them through emit_identifier_maybe_quoted instead of using bare TokenKind::IDENT tokens, and added test cases covering one statement per fixed emitter.

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Many emitters wrote a dynamic identifier (a user-supplied name read from the AST) straight into a raw TokenKind::IDENT token, bypassing the quote aware helper, leading to the quotes always being dropped, which can produce invalid SQL in case of reserved keywords or multi-token identifiers, or change the semantics if the quoted identifier is case-sensitive (e.g. quoted schema names are case-sensitive).

1. parse-fail: reserved keyword emitted bare => invalid SQL

-- input
ALTER TABLE t1 SET SCHEMA "primary";

-- formatter output (un-fixed): quotes dropped
alter table t1 set schema primary;
--                          ^^^^^^^ "primary" is a reserved keyword => syntax error at or near "primary"

2. ast-mismatch: mixed-case name folded => different meaning

-- input
REINDEX SCHEMA "MySchema";

-- formatter output (un-fixed): quotes dropped, case folded
reindex schema myschema;
--             ^^^^^^^^ now refers to a different schema (parses fine, wrong semantics)
Full cargo test output against un-fixed code
running 2 tests
test test_single__index_elem_reserved_keyword_0 ... FAILED
test test_multi__reserved_keyword_idents ... FAILED

failures:

---- test_single__index_elem_reserved_keyword_0 stdout ----
Original content:
CREATE UNIQUE INDEX index_user_emails_on_user_id_and_primary ON public.user_emails USING btree (user_id, "primary") WHERE "primary";

Parsed AST: IndexStmt(
    IndexStmt {
        idxname: "index_user_emails_on_user_id_and_primary",
        relation: Some(
            RangeVar {
                catalogname: "",
                schemaname: "public",
                relname: "user_emails",
                inh: true,
                relpersistence: "p",
                alias: None,
                location: 64,
            },
        ),
        access_method: "btree",
        table_space: "",
        index_params: [
            Node {
                node: Some(
                    IndexElem(
                        IndexElem {
                            name: "user_id",
                            expr: None,
                            indexcolname: "",
                            collation: [],
                            opclass: [],
                            opclassopts: [],
                            ordering: SortbyDefault,
                            nulls_ordering: SortbyNullsDefault,
                        },
                    ),
                ),
            },
            Node {
                node: Some(
                    IndexElem(
                        IndexElem {
                            name: "primary",
                            expr: None,
                            indexcolname: "",
                            collation: [],
                            opclass: [],
                            opclassopts: [],
                            ordering: SortbyDefault,
                            nulls_ordering: SortbyNullsDefault,
                        },
                    ),
                ),
            },
        ],
        index_including_params: [],
        options: [],
        where_clause: Some(
            Node {
                node: Some(
                    ColumnRef(
                        ColumnRef {
                            fields: [
                                Node {
                                    node: Some(
                                        String(
                                            String {
                                                sval: "primary",
                                            },
                                        ),
                                    ),
                                },
                            ],
                            location: 122,
                        },
                    ),
                ),
            },
        ),
        exclude_op_names: [],
        idxcomment: "",
        index_oid: 0,
        old_number: 0,
        old_create_subid: 0,
        old_first_relfilelocator_subid: 0,
        unique: true,
        nulls_not_distinct: false,
        primary: false,
        isconstraint: false,
        deferrable: false,
        initdeferred: false,
        transformed: false,
        concurrent: false,
        if_not_exists: false,
        reset_default_tblspc: false,
    },
)
Formatted content (width=80):
create unique
index "index_user_emails_on_user_id_and_primary"
on public.user_emails
using btree
(
  user_id,
  primary
)
where
  "primary";

thread 'test_single__index_elem_reserved_keyword_0' (978946) panicked at crates/pgls_pretty_print/tests/tests.rs:82:56:
Failed to parse SQL: Parse("syntax error at or near \"primary\"")

---- test_multi__reserved_keyword_idents stdout ----
Parsed AST: CreateStmt(
    CreateStmt {
        relation: Some(
            RangeVar {
                catalogname: "",
                schemaname: "",
                relname: "primary",
                inh: true,
                relpersistence: "p",
                alias: None,
                location: 13,
            },
        ),
        table_elts: [
            Node {
                node: Some(
                    ColumnDef(
                        ColumnDef {
                            colname: "x",
                            type_name: Some(
                                TypeName {
                                    names: [
                                        Node {
                                            node: Some(
                                                String(
                                                    String {
                                                        sval: "pg_catalog",
                                                    },
                                                ),
                                            ),
                                        },
                                        Node {
                                            node: Some(
                                                String(
                                                    String {
                                                        sval: "int4",
                                                    },
                                                ),
                                            ),
                                        },
                                    ],
                                    type_oid: 0,
                                    setof: false,
                                    pct_type: false,
                                    typmods: [],
                                    typemod: -1,
                                    array_bounds: [],
                                    location: 26,
                                },
                            ),
                            compression: "",
                            inhcount: 0,
                            is_local: true,
                            is_not_null: false,
                            is_from_type: false,
                            storage: "",
                            storage_name: "",
                            raw_default: None,
                            cooked_default: None,
                            identity: "",
                            identity_sequence: None,
                            coll_clause: None,
                            coll_oid: 0,
                            constraints: [],
                            fdwoptions: [],
                            location: 24,
                        },
                    ),
                ),
            },
        ],
        inh_relations: [],
        partbound: None,
        partspec: None,
        of_typename: None,
        constraints: [],
        options: [],
        oncommit: OncommitNoop,
        tablespacename: "",
        access_method: "",
        if_not_exists: false,
    },
)
Parsed AST: CreateStmt(
    CreateStmt {
        relation: Some(
            RangeVar {
                catalogname: "",
                schemaname: "",
                relname: "t1",
                inh: true,
                relpersistence: "p",
                alias: None,
                location: 13,
            },
        ),
        table_elts: [
            Node {
                node: Some(
                    ColumnDef(
                        ColumnDef {
                            colname: "primary",
                            type_name: Some(
                                TypeName {
                                    names: [
                                        Node {
                                            node: Some(
                                                String(
                                                    String {
                                                        sval: "pg_catalog",
                                                    },
                                                ),
                                            ),
                                        },
                                        Node {
                                            node: Some(
                                                String(
                                                    String {
                                                        sval: "int4",
                                                    },
                                                ),
                                            ),
                                        },
                                    ],
                                    type_oid: 0,
                                    setof: false,
                                    pct_type: false,
                                    typmods: [],
                                    typemod: -1,
                                    array_bounds: [],
                                    location: 27,
                                },
                            ),
                            compression: "",
                            inhcount: 0,
                            is_local: true,
                            is_not_null: false,
                            is_from_type: false,
                            storage: "",
                            storage_name: "",
                            raw_default: None,
                            cooked_default: None,
                            identity: "",
                            identity_sequence: None,
                            coll_clause: None,
                            coll_oid: 0,
                            constraints: [],
                            fdwoptions: [],
                            location: 17,
                        },
                    ),
                ),
            },
        ],
        inh_relations: [],
        partbound: None,
        partspec: None,
        of_typename: None,
        constraints: [],
        options: [],
        oncommit: OncommitNoop,
        tablespacename: "",
        access_method: "",
        if_not_exists: false,
    },
)
Parsed AST: CreateStmt(
    CreateStmt {
        relation: Some(
            RangeVar {
                catalogname: "",
                schemaname: "",
                relname: "t2",
                inh: true,
                relpersistence: "p",
                alias: None,
                location: 13,
            },
        ),
        table_elts: [
            Node {
                node: Some(
                    ColumnDef(
                        ColumnDef {
                            colname: "x",
                            type_name: Some(
                                TypeName {
                                    names: [
                                        Node {
                                            node: Some(
                                                String(
                                                    String {
                                                        sval: "pg_catalog",
                                                    },
                                                ),
                                            ),
                                        },
                                        Node {
                                            node: Some(
                                                String(
                                                    String {
                                                        sval: "int4",
                                                    },
                                                ),
                                            ),
                                        },
                                    ],
                                    type_oid: 0,
                                    setof: false,
                                    pct_type: false,
                                    typmods: [],
                                    typemod: -1,
                                    array_bounds: [],
                                    location: 19,
                                },
                            ),
                            compression: "",
                            inhcount: 0,
                            is_local: true,
                            is_not_null: false,
                            is_from_type: false,
                            storage: "",
                            storage_name: "",
                            raw_default: None,
                            cooked_default: None,
                            identity: "",
                            identity_sequence: None,
                            coll_clause: None,
                            coll_oid: 0,
                            constraints: [
                                Node {
                                    node: Some(
                                        Constraint(
                                            Constraint {
                                                contype: ConstrCheck,
                                                conname: "primary",
                                                deferrable: false,
                                                initdeferred: false,
                                                skip_validation: false,
                                                initially_valid: true,
                                                is_no_inherit: false,
                                                raw_expr: Some(
                                                    Node {
                                                        node: Some(
                                                            AExpr(
                                                                AExpr {
                                                                    kind: AexprOp,
                                                                    name: [
                                                                        Node {
                                                                            node: Some(
                                                                                String(
                                                                                    String {
                                                                                        sval: ">",
                                                                                    },
                                                                                ),
                                                                            ),
                                                                        },
                                                                    ],
                                                                    lexpr: Some(
                                                                        Node {
                                                                            node: Some(
                                                                                ColumnRef(
                                                                                    ColumnRef {
                                                                                        fields: [
                                                                                            Node {
                                                                                                node: Some(
                                                                                                    String(
                                                                                                        String {
                                                                                                            sval: "x",
                                                                                                        },
                                                                                                    ),
                                                                                                ),
                                                                                            },
                                                                                        ],
                                                                                        location: 51,
                                                                                    },
                                                                                ),
                                                                            ),
                                                                        },
                                                                    ),
                                                                    rexpr: Some(
                                                                        Node {
                                                                            node: Some(
                                                                                AConst(
                                                                                    AConst {
                                                                                        isnull: false,
                                                                                        location: 55,
                                                                                        val: Some(
                                                                                            Ival(
                                                                                                Integer {
                                                                                                    ival: 0,
                                                                                                },
                                                                                            ),
                                                                                        ),
                                                                                    },
                                                                                ),
                                                                            ),
                                                                        },
                                                                    ),
                                                                    location: 53,
                                                                },
                                                            ),
                                                        ),
                                                    },
                                                ),
                                                cooked_expr: "",
                                                inhcount: 0,
                                                nulls_not_distinct: false,
                                                keys: [],
                                                including: [],
                                                exclusions: [],
                                                options: [],
                                                indexname: "",
                                                indexspace: "",
                                                reset_default_tblspc: false,
                                                access_method: "",
                                                where_clause: None,
                                                pktable: None,
                                                fk_attrs: [],
                                                pk_attrs: [],
                                                fk_matchtype: "",
                                                fk_upd_action: "",
                                                fk_del_action: "",
                                                fk_del_set_cols: [],
                                                old_conpfeqop: [],
                                                old_pktable_oid: 0,
                                                location: 23,
                                            },
                                        ),
                                    ),
                                },
                            ],
                            fdwoptions: [],
                            location: 17,
                        },
                    ),
                ),
            },
        ],
        inh_relations: [],
        partbound: None,
        partspec: None,
        of_typename: None,
        constraints: [],
        options: [],
        oncommit: OncommitNoop,
        tablespacename: "",
        access_method: "",
        if_not_exists: false,
    },
)
Parsed AST: AlterObjectSchemaStmt(
    AlterObjectSchemaStmt {
        object_type: ObjectTable,
        relation: Some(
            RangeVar {
                catalogname: "",
                schemaname: "",
                relname: "t1",
                inh: true,
                relpersistence: "p",
                alias: None,
                location: 12,
            },
        ),
        object: None,
        newschema: "primary",
        missing_ok: false,
    },
)
Failed to parse formatted SQL. Error: Parse("syntax error at or near \"primary\"")
Statement index: 466
Formatted SQL:
alter table t1 set schema primary;

thread 'test_multi__reserved_keyword_idents' (978945) panicked at crates/pgls_pretty_print/tests/tests.rs:171:17:
Failed to parse formatted SQL: Parse("syntax error at or near \"primary\"")


failures:
    test_multi__reserved_keyword_idents
    test_single__index_elem_reserved_keyword_0

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 512 filtered out; finished in 0.00s

error: test failed, to rerun pass `-p pgls_pretty_print --test tests`

Note: cargo test panics on the first broken statement per file, so it
reports one representative failure. The corpus_sweep run below enumerates all 46.

Corpus sweep failures
fixture category widths source error
parse-fail/alter_database_set_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_event_trig_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_extension_contents_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_object_depends_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_op_family_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_publication_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_publication_stmt_0002 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_role_set_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_table_move_all_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_table_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_table_stmt_0002 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/alter_object_schema_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/close_portal_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/cluster_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_am_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_extension_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_fdw_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_foreign_table_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/index_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_op_class_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_op_family_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_publication_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/rule_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_transform_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_trig_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/index_stmt_0002 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/index_stmt_0003 parse-fail 80,100 /tmp/demo-fixtures/index_elem_reserved_keyword_0.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/create_user_mapping_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/deallocate_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/dropdb_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/drop_table_space_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/drop_user_mapping_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/execute_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/fetch_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/import_foreign_schema_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/listen_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/prepare_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
ast-mismatch/reindex_stmt_0001 ast-mismatch 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Formatter (beta): This statement type is not fully supported yet. Formatting may alter semantics.
parse-fail/reindex_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/select_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/select_stmt_0002 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "Case"
ast-mismatch/select_stmt_0001 ast-mismatch 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Formatter (beta): This statement type is not fully supported yet. Formatting may alter semantics.
parse-fail/select_stmt_0003 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/unlisten_stmt_0001 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"
parse-fail/select_stmt_0004 parse-fail 80,100 /tmp/demo-fixtures/reserved_keyword_idents.sql Failed to parse formatted output: Formatted SQL failed to parse: Invalid statement: syntax error at or near "primary"

What is the new behavior?

Routed emitters with dynamic identifiers through the emit_identifier_maybe_quoted helper.

Additional context

I have a small corpus_sweep helper, with which I compared the ASTs of the SQL files I downloaded against the produced SQL of the pretty-print. I wasn't sure whether to include it in the PR, but it allowed me to find the first instance of the bug, from which I looked for similar issues!

…across all emitters

Several emitters printed identifiers as bare tokens instead of using the
quote-aware helper, so names that require quoting (reserved keywords,
multi-word, mixed-case) lost their qutes and produced invalid or
semantically different SQL.

Similar to (supabase-community#749), routed them through `emit_identifier_maybe_quoted`
instead of using bare `TokenKind::IDENT` tokens, and added test cases
covering one statement per fixed emitter.
@psteinroe
Copy link
Copy Markdown
Collaborator

thanks for the contribution! it looks good to me, but can you check why the AST comparison that is embedded into the formatter entry point didn't catch these cases?

@JohannesKnopp
Copy link
Copy Markdown
Contributor Author

Hey, the ast-missmatch cases are when BetaUnsupported got thrown, I didn't realize this was basically already the guard firing. The emitter was outputting the wrong thing, and I think I got confused about what the error meant. Sorry for the misleading demo.

@JohannesKnopp JohannesKnopp marked this pull request as draft June 3, 2026 09:52
@JohannesKnopp
Copy link
Copy Markdown
Contributor Author

Coverting to draft again, I ran a fuzzer on my corpus which found a few more emitters with the same bug

@psteinroe
Copy link
Copy Markdown
Collaborator

No worries! Thanks for hardening the formatter, would love to stabilise and properly release it but my time is limited at the moment. Greatly appreciate the support!

@JohannesKnopp
Copy link
Copy Markdown
Contributor Author

Think I found a bug where the original/formatted AST is the same, even though the meaning is not the same:

poc.sh:

#!/usr/bin/env bash
set -euo pipefail

cd "$(dirname "$0")"
WORK="$(mktemp -d)"
trap 'rm -rf "$WORK"' EXIT

ORIG="$WORK/orig.sql"
FMT="$WORK/fmt.sql"

# The ORIGINAL input: case-sensitive identifiers, correctly quoted.
cat > "$ORIG" <<'SQL'
SET search_path = "MySchema";
SET ROLE "MyRole";
SET search_path = "MySchema", public;
SET xmloption = DOCUMENT;
SQL

cp "$ORIG" "$FMT"

cargo build -q -p pgls_cli
cargo build -q -p pgls_pretty_print --example dump_ast

CLI=./target/debug/postgres-language-server
DUMP() { cargo run -q -p pgls_pretty_print --example dump_ast -- "$1" "$2"; }

echo
echo "### Running CLI: format --write on the copy"
"$CLI" format --write "$FMT" || true

echo
echo "===================== INPUT (orig.sql) ====================="
cat "$ORIG"
echo "================== CLI OUTPUT (fmt.sql) ===================="
cat "$FMT"

echo
echo "############################################################"
echo "# 1) RAW ASTs  --  orig  vs  CLI-formatted"
echo "#    A non-empty diff here proves the CLI changed the AST."
echo "############################################################"
if diff -u <(DUMP "$ORIG" raw) <(DUMP "$FMT" raw); then
  echo ">>> RAW ASTs are IDENTICAL (no bug)"
else
  echo ">>> RAW ASTs DIFFER  ==>  the CLI silently changed the parsed meaning."
fi

echo
echo "############################################################"
echo "# 2) NORMALIZED ASTs  --  orig  vs  CLI-formatted"
echo "#    An EMPTY diff here is why the guard returned Ok and"
echo "#    let the corrupting rewrite through."
echo "############################################################"
if diff -u <(DUMP "$ORIG" normalized) <(DUMP "$FMT" normalized); then
  echo ">>> NORMALIZED ASTs are IDENTICAL  ==>  guard sees no difference  ==>  Ok."
else
  echo ">>> NORMALIZED ASTs DIFFER (guard would have caught it)"
fi

Output:

### Running CLI: format --write on the copy
Processed 1 file in 1005µs. Fixed 1 file.

===================== INPUT (orig.sql) =====================
SET search_path = "MySchema";
SET ROLE "MyRole";
SET search_path = "MySchema", public;
SET xmloption = DOCUMENT;
================== CLI OUTPUT (fmt.sql) ====================
set search_path to MySchema;

set role to MyRole;

set search_path to MySchema, public;

set xmloption = document;
############################################################
# 1) RAW ASTs  --  orig  vs  CLI-formatted
#    A non-empty diff here proves the CLI changed the AST.
############################################################
--- /dev/fd/63  2026-06-03 16:18:58.045772696 +0200
+++ /dev/fd/62  2026-06-03 16:18:58.045772696 +0200
@@ -9,11 +9,11 @@
                     AConst(
                         AConst {
                             isnull: false,
-                            location: 18,
+                            location: 19,
                             val: Some(
                                 Sval(
                                     String {
-                                        sval: "MySchema",
+                                        sval: "myschema",
                                     },
                                 ),
                             ),
@@ -37,11 +37,11 @@
                     AConst(
                         AConst {
                             isnull: false,
-                            location: 9,
+                            location: 12,
                             val: Some(
                                 Sval(
                                     String {
-                                        sval: "MyRole",
+                                        sval: "myrole",
                                     },
                                 ),
                             ),
@@ -65,11 +65,11 @@
                     AConst(
                         AConst {
                             isnull: false,
-                            location: 18,
+                            location: 19,
                             val: Some(
                                 Sval(
                                     String {
-                                        sval: "MySchema",
+                                        sval: "myschema",
                                     },
                                 ),
                             ),
@@ -82,7 +82,7 @@
                     AConst(
                         AConst {
                             isnull: false,
-                            location: 30,
+                            location: 29,
                             val: Some(
                                 Sval(
                                     String {
>>> RAW ASTs DIFFER  ==>  the CLI silently changed the parsed meaning.

############################################################
# 2) NORMALIZED ASTs  --  orig  vs  CLI-formatted
#    An EMPTY diff here is why the guard returned Ok and
#    let the corrupting rewrite through.
############################################################
>>> NORMALIZED ASTs are IDENTICAL  ==>  guard sees no difference  ==>  Ok.

I think the issue is in crates/pgls_pretty_print/src/normalize.rs, lines 407-420, where it normalizes potentially case-sensitive values before comparing the ASTs:

            NodeMut::VariableSetStmt(n) => {
                // SET statement values are case-insensitive in PostgreSQL
                // e.g., SET xmloption = DOCUMENT vs SET xmloption = document
                for arg in &mut (*n).args {
                    if let Some(NodeEnum::AConst(aconst)) = arg.node.as_mut() {
                        aconst.location = 0;
                        if let Some(pgls_query::protobuf::a_const::Val::Sval(s)) =
                            aconst.val.as_mut()
                        {
                            s.sval = s.sval.to_lowercase();  // "MySchema" and "myschema" become myschema
                        }
                    }
                }
            }

This bug is a bit different, so I wasn't sure if it fits in this branch/PR, @psteinroe how would you handle it?

@psteinroe
Copy link
Copy Markdown
Collaborator

@JohannesKnopp thanks for the repro! a separate pr would be great for the normalisation fix. :)

@JohannesKnopp
Copy link
Copy Markdown
Contributor Author

@psteinroe Sounds good, in that case this PR would be ready from my side!

@JohannesKnopp JohannesKnopp marked this pull request as ready for review June 3, 2026 15:55
@psteinroe psteinroe self-requested a review June 3, 2026 16:08
@psteinroe psteinroe enabled auto-merge (squash) June 3, 2026 16:09
@psteinroe psteinroe merged commit 57f5e2d into supabase-community:main Jun 3, 2026
9 checks passed
@JohannesKnopp JohannesKnopp deleted the fix/index-elem-reserved-keyword-quoting branch June 5, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants