Skip to content

Render single spaces in ALTER visibility, FORCE and row level security actions - #2819

Open
minleejae wants to merge 1 commit into
JSQLParser:masterfrom
minleejae:fix/alter-render-spacing-20261014
Open

minleejae wants to merge 1 commit into
JSQLParser:masterfrom
minleejae:fix/alter-render-spacing-20261014

Conversation

@minleejae

@minleejae minleejae commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Follow-up to #2792. More renderers emit stray spaces. Today:

  • ALTER TABLE t ALTER COLUMN c SET INVISIBLE renders SET INVISIBLE (also SET VISIBLE, and the form without COLUMN).
  • ALTER TABLE t ALTER INDEX i INVISIBLE renders ALTER INDEX i INVISIBLE.
  • ALTER TABLE t FORCE ends with a space, and FORCE, ALGORITHM = INPLACE renders FORCE , ALGORITHM = INPLACE.
  • PostgreSQL ALTER TABLE t {ENABLE | DISABLE | FORCE | NO FORCE} ROW LEVEL SECURITY ends with a space.
  • ALTER TABLE t MODIFY (c INT) renders MODIFY ( c INT ) . ADD CONSTRAINT UNIQUE KEY uk (a) renders uk (a).
  • The Index of CREATE INDEX ON t (a) renders (a) when printed on its own.
  • MySQL DEFAULT (JSON_ARRAY()) renders DEFAULT (JSON_ARRAY( ) ), and JSON_OBJECT() renders JSON_OBJECT( ) .

The SQL still reparses, but the extra spaces differ from the input.

Current design (As-is)

  • ColumnSetVisibility appends " SET " and then " VISIBLE" or " INVISIBLE".
  • Index puts two spaces before its trailing index spec when it has no columns. It also puts a space before the column list even when nothing precedes it.
  • The general AlterExpression body appends a space after the operation keyword even when no payload follows. The row level security keywords add the same space.
  • A bracketed single column definition is wrapped in " ( " and " ) ". The ADD CONSTRAINT <type> <name> header ends with a space before the column list's own separator.
  • JSON_ARRAY and JSON_OBJECT always open with "( " and close with ") " or " ) ", even with no arguments.

Target design (To-be)

  • Separate these words with exactly one space and add no leading or trailing space:
    • ALTER COLUMN c SET INVISIBLE, ALTER INDEX i INVISIBLE, FORCE and ... ROW LEVEL SECURITY;
    • MODIFY (c INT), ADD CONSTRAINT UNIQUE KEY uk (a);
    • an Index with no declaration renders (a).
  • JSON_ARRAY() and JSON_OBJECT() render without inner spaces. No space follows the closing parenthesis. Calls with arguments keep their current inner spacing.
  • The legacy EXCHANGE PARTITION ... WITH VALIDATION rendering of a plain AlterExpression drops its trailing space.
  • Rendered SQL is otherwise unchanged. The SQL renderer and StatementDeParser produce the same text. The grammar is unchanged.

Validation

  • DeparseWhitespaceTest (added in Render single spaces in DROP PRIMARY KEY, RENAME TABLE and indexes #2792) now also:
    • compares toString() and StatementDeParser output exactly for the forms above in MySQL, PostgreSQL and default mode;
    • checks that JSON_ARRAY/JSON_OBJECT with arguments or ON NULL clauses render no double space;
    • checks ColumnSetVisibility, a constructed Index and a constructed EXCHANGE PARTITION action directly.
  • No existing assertion encoded these spaces, so no existing test changed.
  • Java 17 offline Gradle spotlessApply check: 10,382 tests, 10,357 passed, 25 skipped, zero failures/errors.
  • Compared with master (bdf8ac31) on a corpus of 635 MySQL 8.4 and PostgreSQL 18 DDL/DCL statements, each accepted by the server, with and without a dialect: no regressions. Only 6 MySQL and 3 PostgreSQL statements of the forms above change their toString(), in both dialect and default mode. In CREATE INDEX ON ..., the contained index renders (name) instead of (name).
  • MySQL 8.4 and PostgreSQL 18.3: every corpus statement this branch parses ran as written on one server and as rendered by toString() on a second, identical server, in the same order. All 205 MySQL and 328 PostgreSQL statements give the same result (success, or the same error).

Integration validation

Applied #2806–#2819 together on master (bdf8ac31), with the stacked PRs on top of their bases (#2806 → #2807 → #2808, #2806 → #2809 → #2810 and #2811 → #2812).

  • Each branch merges cleanly into master. Between the branches, conflicts are limited to adjacent additions (README/index rows, usage sections, imports, visitor and feature lists, one enum line and one alias guard).
  • The integrated build passes Java 17 offline Gradle spotlessApply check: 11,494 tests, 11,469 passed, 25 skipped, zero failures/errors. The grammar choice conflict check passes.
  • On a corpus of 635 MySQL 8.4 and PostgreSQL 18 DDL/DCL statements, each accepted by the server, the integrated build parses 265 of 271 MySQL statements (master: 206) and 363 of 364 PostgreSQL statements (master: 328). Every MySQL 8.4 non-reserved word and PostgreSQL 18 unreserved or column-name keyword parses as a column and table name.
  • On paired MySQL 8.4 and PostgreSQL 18.3 servers, all 263 MySQL and 362 PostgreSQL statements rendered by the integrated build give the same results as the originals.

…curity actions

Rendered SQL had stray spaces in several ALTER TABLE actions:
SET  INVISIBLE, ALTER INDEX i  INVISIBLE, a trailing space after
FORCE and [NO] FORCE/ENABLE/DISABLE ROW LEVEL SECURITY, MODIFY  ( c INT ),
and ADD CONSTRAINT UNIQUE KEY uk  (a). An index without a declaration
printed a leading space, and argument-less JSON_ARRAY()/JSON_OBJECT()
rendered as JSON_ARRAY( ) and JSON_OBJECT(  ) with a trailing space.

Separate these words with exactly one space and add no leading or
trailing space. toString() and StatementDeParser stay identical.

Signed-off-by: minleejae <mmj9808@gmail.com>

This branch has not been deployed

No deployments
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