Skip to content

fix(pretty-print): preserve PostgreSQL syntax in five printer edge cases - #799

Open
edjubert wants to merge 6 commits into
supabase-community:mainfrom
edjubert:edjubert/printer-output-fixes
Open

edjubert wants to merge 6 commits into
supabase-community:mainfrom
edjubert:edjubert/printer-output-fixes

Conversation

@edjubert

Copy link
Copy Markdown

What

Five independent output defects in the printer.
Four of them made the beta round-trip guard refuse the statement (Normalized ASTs differ after formatting), so the user silently got no formatting at all.
The fifth is a layout regression.

1. ALTER TABLE ... ADD COLUMN IF NOT EXISTS

missing_ok was never emitted, so the statement came back without IF NOT EXISTS and the guard rejected it.

ALTER TABLE s.t
	ADD COLUMN IF NOT EXISTS row_id TEXT,
	ADD COLUMN IF NOT EXISTS designation_source TEXT;

Dropping the flag would turn an idempotent migration into one that fails on its second run, so the guard was right to refuse - the fix is to emit it.

2. UNIQUE NULLS NOT DISTINCT on a table constraint

nulls_not_distinct was emitted for CREATE INDEX but not for a table constraint, where the qualifier sits between the keyword and the column list:

CREATE TABLE s.expense_types (
	expense_type_legacy_id TEXT,
	legacy_branch_code TEXT,
	UNIQUE NULLS NOT DISTINCT (expense_type_legacy_id, legacy_branch_code)
);

Losing it would let two NULL rows coexist where the schema forbids it.

3. A boolean expression as the argument of a boolean test

AND, OR and NOT bind more loosely than the postfix IS test, so (a OR b) IS TRUE was printed as a or b is true, which reparses as a OR (b IS TRUE).
The argument is now parenthesised when - and only when - it is a BoolExpr.
Every other argument kind binds tighter and needs nothing.

WHERE (units.has_history OR units.has_shares OR units.has_calls) IS TRUE
  AND (units.is_active AND units.is_visible) IS NOT FALSE

4. DISTINCT ON parentheses

The opening parenthesis emitted a soft line but the closing one did not, so a broken list left the ) glued to its last element:

-- before
select distinct on (
  a,
  b)
  a,
  b

-- after
select distinct on (a, b)
  a,
  b

The clause now also gets its own group, so it can stay on one line while the target list breaks - the usual shape of a DISTINCT ON query.
This changes several existing snapshots (groupingsets, memoize, select_distinct_on, subselect, tsrf), all in that direction.

5. typeCase on a qualified type name

CAST(t.id AS public.object_id) went through the plain identifier path, so typeCase did not apply to it while it applied to bigint.
A new emit_type_identifier_maybe_quoted emits a TYPE_IDENT token for names that can be written unquoted, and falls back to the verbatim identifier for names that need quotes - "MyType" and "MYTYPE" are two different types in PostgreSQL, so recasing them would change which type is referenced.

Tests

Three new fixtures (qualified_type_cast, distinct_on_long_list, unique_constraint_nulls_not_distinct), snapshots for two fixtures that had none (alter_table_add_column_if_not_exists, boolean_test_bool_expr), and two unit tests on the new emitter covering the quoted/unquoted split.

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.

1 participant