From 29dcb3a34ec70e9ad87b856c09c506936ba4015d Mon Sep 17 00:00:00 2001 From: minleejae Date: Wed, 14 Oct 2026 12:55:38 +0900 Subject: [PATCH] feat: extend PostgreSQL COMMENT ON targets Model the remaining common PostgreSQL COMMENT ON object kinds with the existing CommentTarget: TRIGGER, POLICY and RULE within a table; PROCEDURE, ROUTINE and AGGREGATE signatures through RoutineReference; CAST; and ACCESS METHOD, COLLATION, DATABASE, EVENT TRIGGER, EXTENSION, FOREIGN DATA WRAPPER, FOREIGN TABLE, [PROCEDURAL] LANGUAGE, PUBLICATION, ROLE, SERVER, STATISTICS, SUBSCRIPTION and TABLESPACE. The grammar reads the kind first and lets it select the reference shape. Kind words without a token stay non-reserved. Shared routine signatures now accept ordered-set aggregates without direct arguments, such as mode(ORDER BY anyelement). Signed-off-by: minleejae --- .../sf/jsqlparser/parser/feature/Feature.java | 18 ++ .../statement/comment/CommentTarget.java | 88 +++++- .../validation/feature/PostgresqlVersion.java | 10 + .../validator/CommentValidator.java | 42 +++ .../net/sf/jsqlparser/parser/JSqlParserCC.jjt | 79 ++++-- src/site/sphinx/usage.rst | 54 ++-- .../statement/comment/CommentTargetTest.java | 253 ++++++++++++++++-- .../validator/CommentValidatorTest.java | 41 ++- 8 files changed, 523 insertions(+), 62 deletions(-) diff --git a/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java b/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java index 0d3d080fb..0f8e63843 100644 --- a/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java +++ b/src/main/java/net/sf/jsqlparser/parser/feature/Feature.java @@ -647,6 +647,24 @@ public enum Feature { /** Additional structured catalog targets of COMMENT ON. */ commentOnIndex, commentOnSchema, commentOnSequence, commentOnDomain, commentOnType, commentOnMaterializedView, commentOnFunction, commentOnConstraint, + /** COMMENT ON TRIGGER, POLICY or RULE within a table. */ + commentOnTrigger, commentOnPolicy, commentOnRule, + + /** COMMENT ON PROCEDURE, ROUTINE, AGGREGATE or CAST. */ + commentOnProcedure, commentOnRoutine, commentOnAggregate, commentOnCast, + + /** COMMENT ON COLLATION, FOREIGN TABLE or STATISTICS. */ + commentOnCollation, commentOnForeignTable, commentOnStatistics, + + /** COMMENT ON DATABASE, ROLE or TABLESPACE. */ + commentOnDatabase, commentOnRole, commentOnTablespace, + + /** COMMENT ON ACCESS METHOD, EXTENSION, FOREIGN DATA WRAPPER, LANGUAGE or SERVER. */ + commentOnAccessMethod, commentOnExtension, commentOnForeignDataWrapper, commentOnLanguage, commentOnServer, + + /** COMMENT ON EVENT TRIGGER, PUBLICATION or SUBSCRIPTION. */ + commentOnEventTrigger, commentOnPublication, commentOnSubscription, + /** * SQL "DESCRIBE" statement is allowed * diff --git a/src/main/java/net/sf/jsqlparser/statement/comment/CommentTarget.java b/src/main/java/net/sf/jsqlparser/statement/comment/CommentTarget.java index 7269d29a6..73ee4e359 100644 --- a/src/main/java/net/sf/jsqlparser/statement/comment/CommentTarget.java +++ b/src/main/java/net/sf/jsqlparser/statement/comment/CommentTarget.java @@ -13,11 +13,33 @@ import java.util.function.Consumer; import net.sf.jsqlparser.schema.Table; import net.sf.jsqlparser.statement.RoutineReference; +import net.sf.jsqlparser.statement.create.table.ColDataType; /** A catalog object addressed by COMMENT, rather than a function invocation or a query. */ public class CommentTarget implements Serializable { public enum Kind { - INDEX, SCHEMA, SEQUENCE, DOMAIN, TYPE, MATERIALIZED_VIEW, FUNCTION, CONSTRAINT + INDEX, SCHEMA, SEQUENCE, DOMAIN, TYPE, MATERIALIZED_VIEW, FUNCTION, CONSTRAINT, + + TRIGGER, POLICY, RULE, PROCEDURE, ROUTINE, AGGREGATE, CAST, + + COLLATION, FOREIGN_TABLE, STATISTICS, DATABASE, ROLE, TABLESPACE, ACCESS_METHOD, EXTENSION, + + FOREIGN_DATA_WRAPPER, LANGUAGE, SERVER, EVENT_TRIGGER, PUBLICATION, SUBSCRIPTION; + + /** + * Whether the target is a routine signature, exposed by {@link CommentTarget#getRoutine()}. + */ + public boolean isRoutine() { + return this == FUNCTION || this == PROCEDURE || this == ROUTINE || this == AGGREGATE; + } + + /** + * Whether the target is named within the table or domain of + * {@link CommentTarget#getRelation()}. + */ + public boolean isOwnedByRelation() { + return this == CONSTRAINT || this == TRIGGER || this == POLICY || this == RULE; + } } private Kind kind; @@ -25,6 +47,9 @@ public enum Kind { private RoutineReference routine; private Table relation; private boolean onDomain; + private boolean procedural; + private ColDataType sourceType; + private ColDataType targetType; public Kind getKind() { return kind; @@ -34,7 +59,10 @@ public void setKind(Kind kind) { this.kind = kind; } - /** The object's identifier; using Table preserves the individual name components. */ + /** + * The object's identifier; using Table preserves the individual name components. Routine and + * cast targets have no name. + */ public Table getName() { return name; } @@ -51,7 +79,10 @@ public void setRoutine(RoutineReference routine) { this.routine = routine; } - /** The table or domain owning a constraint, distinguished by {@link #isOnDomain()}. */ + /** + * The table owning a constraint, trigger, policy or rule, or the domain owning a constraint, + * distinguished by {@link #isOnDomain()}. + */ public Table getRelation() { return relation; } @@ -68,23 +99,62 @@ public void setOnDomain(boolean onDomain) { this.onDomain = onDomain; } - /** Returns only an explicitly named table/view, never an index, type, function or domain. */ + /** Whether a {@link Kind#LANGUAGE} target was written as PROCEDURAL LANGUAGE. */ + public boolean isProcedural() { + return procedural; + } + + public void setProcedural(boolean procedural) { + this.procedural = procedural; + } + + /** The source type of a {@link Kind#CAST} target. */ + public ColDataType getSourceType() { + return sourceType; + } + + public void setSourceType(ColDataType sourceType) { + this.sourceType = sourceType; + } + + /** The target type of a {@link Kind#CAST} target. */ + public ColDataType getTargetType() { + return targetType; + } + + public void setTargetType(ColDataType targetType) { + this.targetType = targetType; + } + + /** + * Returns only an explicitly named table, foreign table or view, never an index, type, + * function, statistics object or domain. + */ public Table getReferencedRelation() { - if (kind == Kind.MATERIALIZED_VIEW) { + if (namesRelation()) { return name; } - return kind == Kind.CONSTRAINT && !onDomain ? relation : null; + return kind != null && kind.isOwnedByRelation() && !onDomain ? relation : null; + } + + private boolean namesRelation() { + return kind == Kind.MATERIALIZED_VIEW || kind == Kind.FOREIGN_TABLE; } public StringBuilder appendTo(StringBuilder builder, Consumer relationWriter) { + if (kind == Kind.LANGUAGE && procedural) { + builder.append("PROCEDURAL "); + } builder.append(kind.name().replace('_', ' ')).append(' '); - if (kind == Kind.FUNCTION) { + if (kind.isRoutine()) { builder.append(routine); - } else if (kind == Kind.MATERIALIZED_VIEW) { + } else if (kind == Kind.CAST) { + builder.append('(').append(sourceType).append(" AS ").append(targetType).append(')'); + } else if (namesRelation()) { relationWriter.accept(name); } else { builder.append(name); - if (kind == Kind.CONSTRAINT) { + if (kind.isOwnedByRelation()) { builder.append(" ON "); if (onDomain) { builder.append("DOMAIN ").append(relation); diff --git a/src/main/java/net/sf/jsqlparser/util/validation/feature/PostgresqlVersion.java b/src/main/java/net/sf/jsqlparser/util/validation/feature/PostgresqlVersion.java index 023a8c0e1..b772e6d1b 100644 --- a/src/main/java/net/sf/jsqlparser/util/validation/feature/PostgresqlVersion.java +++ b/src/main/java/net/sf/jsqlparser/util/validation/feature/PostgresqlVersion.java @@ -95,6 +95,16 @@ public enum PostgresqlVersion implements Version { Feature.commentOnDomain, Feature.commentOnType, Feature.commentOnMaterializedView, Feature.commentOnFunction, Feature.commentOnConstraint, + Feature.commentOnTrigger, Feature.commentOnPolicy, Feature.commentOnRule, + Feature.commentOnProcedure, Feature.commentOnRoutine, + Feature.commentOnAggregate, Feature.commentOnCast, + Feature.commentOnCollation, Feature.commentOnForeignTable, + Feature.commentOnStatistics, Feature.commentOnDatabase, Feature.commentOnRole, + Feature.commentOnTablespace, Feature.commentOnAccessMethod, + Feature.commentOnExtension, Feature.commentOnForeignDataWrapper, + Feature.commentOnLanguage, Feature.commentOnServer, + Feature.commentOnEventTrigger, Feature.commentOnPublication, + Feature.commentOnSubscription, // https://www.postgresql.org/docs/current/sql-createsequence.html Feature.createSequence, diff --git a/src/main/java/net/sf/jsqlparser/util/validation/validator/CommentValidator.java b/src/main/java/net/sf/jsqlparser/util/validation/validator/CommentValidator.java index b5ad37f8b..b737b39d4 100644 --- a/src/main/java/net/sf/jsqlparser/util/validation/validator/CommentValidator.java +++ b/src/main/java/net/sf/jsqlparser/util/validation/validator/CommentValidator.java @@ -46,6 +46,48 @@ private static Feature targetFeature(CommentTarget.Kind kind) { return Feature.commentOnFunction; case CONSTRAINT: return Feature.commentOnConstraint; + case TRIGGER: + return Feature.commentOnTrigger; + case POLICY: + return Feature.commentOnPolicy; + case RULE: + return Feature.commentOnRule; + case PROCEDURE: + return Feature.commentOnProcedure; + case ROUTINE: + return Feature.commentOnRoutine; + case AGGREGATE: + return Feature.commentOnAggregate; + case CAST: + return Feature.commentOnCast; + case COLLATION: + return Feature.commentOnCollation; + case FOREIGN_TABLE: + return Feature.commentOnForeignTable; + case STATISTICS: + return Feature.commentOnStatistics; + case DATABASE: + return Feature.commentOnDatabase; + case ROLE: + return Feature.commentOnRole; + case TABLESPACE: + return Feature.commentOnTablespace; + case ACCESS_METHOD: + return Feature.commentOnAccessMethod; + case EXTENSION: + return Feature.commentOnExtension; + case FOREIGN_DATA_WRAPPER: + return Feature.commentOnForeignDataWrapper; + case LANGUAGE: + return Feature.commentOnLanguage; + case SERVER: + return Feature.commentOnServer; + case EVENT_TRIGGER: + return Feature.commentOnEventTrigger; + case PUBLICATION: + return Feature.commentOnPublication; + case SUBSCRIPTION: + return Feature.commentOnSubscription; default: throw new IllegalArgumentException("Unknown COMMENT target: " + kind); } diff --git a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt index 2210dbe9d..63bec0c60 100644 --- a/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt +++ b/src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt @@ -20291,38 +20291,83 @@ Table CommentObjectName(): : new Table(names.getNames()); } } +/** The object kind decides the shape of the reference that follows it. */ CommentTarget CommentTarget(): { CommentTarget result = new CommentTarget(); + CommentTarget.Kind kind; Table name; - String constraintName; + String objectName; RoutineReference routine; + ColDataType type; } { + kind=CommentTargetKind(result) { result.setKind(kind); } ( - routine=RoutineReference(false) - { result.setKind(CommentTarget.Kind.FUNCTION); result.setRoutine(routine); } + LOOKAHEAD({ kind.isRoutine() }) + routine=RoutineReference(kind == CommentTarget.Kind.AGGREGATE) { result.setRoutine(routine); } | - constraintName=RelObjectName() - { result.setKind(CommentTarget.Kind.CONSTRAINT); result.setName(new Table(constraintName, false)); } + LOOKAHEAD({ kind.isOwnedByRelation() }) + objectName=RelObjectName() { result.setName(new Table(objectName, false)); } - [ LOOKAHEAD({ getToken(1).kind == K_DOMAIN && !".".equals(getToken(2).image) - && getToken(2).kind != K_IS }) { result.setOnDomain(true); } ] + [ LOOKAHEAD({ kind == CommentTarget.Kind.CONSTRAINT && getToken(1).kind == K_DOMAIN + && !".".equals(getToken(2).image) && getToken(2).kind != K_IS }) + { result.setOnDomain(true); } ] name=CommentObjectName() { result.setRelation(name); } | - ( - { result.setKind(CommentTarget.Kind.INDEX); } - | { result.setKind(CommentTarget.Kind.SCHEMA); } - | { result.setKind(CommentTarget.Kind.SEQUENCE); } - | { result.setKind(CommentTarget.Kind.MATERIALIZED_VIEW); } - | { result.setKind(CommentTarget.Kind.DOMAIN); } - | { result.setKind(CommentTarget.Kind.TYPE); } - ) + LOOKAHEAD({ kind == CommentTarget.Kind.CAST }) + "(" type=ColDataType() { result.setSourceType(type); } + type=ColDataType() { result.setTargetType(type); } ")" + | name=CommentObjectName() { result.setName(name); } ) { return result; } } +/** Kind words without a token are matched contextually and remain ordinary identifiers. */ +CommentTarget.Kind CommentTargetKind(CommentTarget target): +{ CommentTarget.Kind kind; } +{ + ( + { kind = CommentTarget.Kind.INDEX; } + | { kind = CommentTarget.Kind.SCHEMA; } + | { kind = CommentTarget.Kind.SEQUENCE; } + | { kind = CommentTarget.Kind.MATERIALIZED_VIEW; } + | { kind = CommentTarget.Kind.DOMAIN; } + | { kind = CommentTarget.Kind.TYPE; } + | { kind = CommentTarget.Kind.FUNCTION; } + | { kind = CommentTarget.Kind.CONSTRAINT; } + | { kind = CommentTarget.Kind.TRIGGER; } + | { kind = CommentTarget.Kind.POLICY; } + | LOOKAHEAD({ isKeywordAhead("RULE") }) ContextualKeyword("RULE") { kind = CommentTarget.Kind.RULE; } + | { kind = CommentTarget.Kind.PROCEDURE; } + | LOOKAHEAD({ isKeywordAhead("ROUTINE") }) ContextualKeyword("ROUTINE") { kind = CommentTarget.Kind.ROUTINE; } + | { kind = CommentTarget.Kind.AGGREGATE; } + | { kind = CommentTarget.Kind.CAST; } + | LOOKAHEAD({ isKeywordAhead("ACCESS") }) ContextualKeyword("ACCESS") ContextualKeyword("METHOD") + { kind = CommentTarget.Kind.ACCESS_METHOD; } + | LOOKAHEAD({ isKeywordAhead("COLLATION") }) ContextualKeyword("COLLATION") { kind = CommentTarget.Kind.COLLATION; } + | { kind = CommentTarget.Kind.DATABASE; } + | { kind = CommentTarget.Kind.EVENT_TRIGGER; } + | LOOKAHEAD({ isKeywordAhead("EXTENSION") }) ContextualKeyword("EXTENSION") { kind = CommentTarget.Kind.EXTENSION; } + | + ( + ContextualKeyword("WRAPPER") { kind = CommentTarget.Kind.FOREIGN_DATA_WRAPPER; } + | { kind = CommentTarget.Kind.FOREIGN_TABLE; } + ) + | LOOKAHEAD({ isKeywordAhead("PROCEDURAL") || isKeywordAhead("LANGUAGE") }) + [ LOOKAHEAD({ isKeywordAhead("PROCEDURAL") }) ContextualKeyword("PROCEDURAL") { target.setProcedural(true); } ] + ContextualKeyword("LANGUAGE") { kind = CommentTarget.Kind.LANGUAGE; } + | LOOKAHEAD({ isKeywordAhead("PUBLICATION") }) ContextualKeyword("PUBLICATION") { kind = CommentTarget.Kind.PUBLICATION; } + | LOOKAHEAD({ isKeywordAhead("ROLE") }) ContextualKeyword("ROLE") { kind = CommentTarget.Kind.ROLE; } + | LOOKAHEAD({ isKeywordAhead("SERVER") }) ContextualKeyword("SERVER") { kind = CommentTarget.Kind.SERVER; } + | LOOKAHEAD({ isKeywordAhead("STATISTICS") }) ContextualKeyword("STATISTICS") { kind = CommentTarget.Kind.STATISTICS; } + | LOOKAHEAD({ isKeywordAhead("SUBSCRIPTION") }) ContextualKeyword("SUBSCRIPTION") { kind = CommentTarget.Kind.SUBSCRIPTION; } + | { kind = CommentTarget.Kind.TABLESPACE; } + ) + { return kind; } +} + /** Consumes a keyword that remains an ordinary identifier outside this grammar context. */ void ContextualKeyword(String expected): { Token keyword; } @@ -21427,7 +21472,9 @@ RoutineReference RoutineReference(boolean aggregate): ( "*" { requireTypeDdlSyntax(aggregate, "Only aggregate signatures accept *"); result.setAllArguments(true); } | - [ argument=RoutineSignatureArgument() { arguments.add(argument); } + // ORDER is non-reserved, so ORDER BY without direct arguments is not an argument name + [ LOOKAHEAD(1, { getToken(1).kind != K_ORDER || getToken(2).kind != K_BY }) + argument=RoutineSignatureArgument() { arguments.add(argument); } ( "," argument=RoutineSignatureArgument() { arguments.add(argument); } )* ] [ { requireTypeDdlSyntax(aggregate, "ORDER BY requires an aggregate signature"); orderBy = new ArrayList(); } argument=RoutineSignatureArgument() { orderBy.add(argument); } diff --git a/src/site/sphinx/usage.rst b/src/site/sphinx/usage.rst index 901608d62..b4515559e 100644 --- a/src/site/sphinx/usage.rst +++ b/src/site/sphinx/usage.rst @@ -1913,36 +1913,54 @@ replacing the expression list clears them. Validators report the separate PostgreSQL COMMENT targets ========================= -``COMMENT ON`` supports ``INDEX``, ``SCHEMA``, ``SEQUENCE``, ``DOMAIN``, ``TYPE``, -``MATERIALIZED VIEW``, ``FUNCTION`` and ``CONSTRAINT`` in addition to the existing -``TABLE``, ``COLUMN`` and ``VIEW`` forms. These unambiguous target forms also parse -without a dialect preset. Tagged dollar strings still require -``Dialect.POSTGRESQL`` or ``withDollarQuotedStringTags(true)``. +``COMMENT ON`` supports these PostgreSQL object kinds in addition to the existing +``TABLE``, ``COLUMN`` and ``VIEW`` forms: + +- ``INDEX``, ``SCHEMA``, ``SEQUENCE``, ``DOMAIN``, ``TYPE``, ``MATERIALIZED VIEW``, + ``FOREIGN TABLE``, ``COLLATION`` and ``STATISTICS`` +- ``ACCESS METHOD``, ``DATABASE``, ``EVENT TRIGGER``, ``EXTENSION``, + ``FOREIGN DATA WRAPPER``, ``[PROCEDURAL] LANGUAGE``, ``PUBLICATION``, ``ROLE``, + ``SERVER``, ``SUBSCRIPTION`` and ``TABLESPACE`` +- ``FUNCTION``, ``PROCEDURE``, ``ROUTINE`` and ``AGGREGATE`` signatures +- ``CONSTRAINT``, ``TRIGGER``, ``POLICY`` and ``RULE`` with their owning table +- ``CAST (source_type AS target_type)`` + +These unambiguous target forms also parse without a dialect preset. Kind words such as +``ROLE``, ``SERVER`` or ``RULE`` are not reserved and remain ordinary identifiers. +Tagged dollar strings still require ``Dialect.POSTGRESQL`` or +``withDollarQuotedStringTags(true)``. The additional targets are exposed through ``Comment.getTarget()`` as a ``CommentTarget``. Its ``Kind`` identifies the object, and ``getName()`` preserves the individual identifier components. ``Table`` is used as the name container; an index or type name does not thereby represent a table dependency. +``isProcedural()`` records ``PROCEDURAL LANGUAGE``. The original ``Comment.getTable()``, ``getColumn()`` and ``getView()`` accessors continue to describe their respective existing forms. ``COMMENT ON FUNCTION app.f(IN value integer) IS 'description'`` uses the shared -``RoutineReference`` in ``getTarget().getRoutine()``. Argument mode, optional name +``RoutineReference`` in ``getTarget().getRoutine()``; ``Kind.isRoutine()`` identifies +these kinds and ``getName()`` is null. Argument mode, optional name and ``ColDataType`` are preserved. An omitted signature has null arguments; -``f()`` has an empty argument list. The signature identifies a function and is not -a function call. +``f()`` has an empty argument list. The signature identifies a routine and is not +a function call. An ``AGGREGATE`` requires a signature and also accepts ``(*)`` and +ordered-set signatures such as ``(float8 ORDER BY integer)`` or ``(ORDER BY integer)``. For ``COMMENT ON CONSTRAINT ck ON app.t IS NULL``, ``getName()`` is the constraint -name and ``getRelation()`` is its owning table. ``ON DOMAIN app.d`` sets -``isOnDomain()`` and stores the domain as the owner instead. ``NULL`` removes the -comment and remains represented by a null ``Comment.getComment()``. - -Table discovery visits tables, column owners, views, materialized views and -table-owned constraints. It does not infer an index's table or treat domains, -sequences, types or functions as tables. Statement visitors visit comment -literals, and custom SQL deparsers can replace the explicit relation or literal. -Feature analysis reports a schema modification. Validation exposes a separate -``commentOn...`` capability for each additional target kind. +name and ``getRelation()`` is its owning table. ``TRIGGER``, ``POLICY`` and ``RULE`` +targets use the same fields; ``Kind.isOwnedByRelation()`` identifies these kinds. +Only a constraint accepts ``ON DOMAIN app.d``, which sets ``isOnDomain()`` and stores +the domain as the owner instead. A ``CAST`` target exposes ``getSourceType()`` and +``getTargetType()``. ``NULL`` removes the comment and remains represented by a null +``Comment.getComment()``. + +Table discovery visits tables, column owners, views, materialized views, foreign +tables and the tables owning constraints, triggers, policies and rules. It does not +infer an index's table or treat domains, sequences, types, routines or other catalog +objects as tables. Statement visitors visit comment literals, and custom SQL deparsers +can replace the explicit relation or literal. Feature analysis reports a schema +modification. Validation exposes a separate ``commentOn...`` capability for each +additional target kind. Current date/time expression metadata ===================================== diff --git a/src/test/java/net/sf/jsqlparser/statement/comment/CommentTargetTest.java b/src/test/java/net/sf/jsqlparser/statement/comment/CommentTargetTest.java index 67f2dddf3..451c0d253 100644 --- a/src/test/java/net/sf/jsqlparser/statement/comment/CommentTargetTest.java +++ b/src/test/java/net/sf/jsqlparser/statement/comment/CommentTargetTest.java @@ -14,7 +14,9 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static net.sf.jsqlparser.test.TestUtils.assertSqlCanBeParsedAndDeparsed; import java.util.ArrayList; +import java.util.EnumSet; import java.util.List; import java.util.Set; import java.util.stream.Stream; @@ -29,6 +31,7 @@ import net.sf.jsqlparser.statement.StatementVisitorAdapter; import net.sf.jsqlparser.statement.Statements; import net.sf.jsqlparser.statement.StmtFeature; +import net.sf.jsqlparser.statement.create.table.ColDataType; import net.sf.jsqlparser.statement.select.SelectVisitorAdapter; import net.sf.jsqlparser.util.TablesNamesFinder; import net.sf.jsqlparser.util.deparser.ExpressionDeParser; @@ -41,15 +44,30 @@ import org.junit.jupiter.params.provider.ValueSource; class CommentTargetTest { + // PostgreSQL accepts schema-qualified names only for these kinds + private static final Set SCHEMA_QUALIFIED = EnumSet.of( + CommentTarget.Kind.INDEX, CommentTarget.Kind.SEQUENCE, CommentTarget.Kind.DOMAIN, + CommentTarget.Kind.TYPE, CommentTarget.Kind.MATERIALIZED_VIEW, + CommentTarget.Kind.FOREIGN_TABLE, CommentTarget.Kind.COLLATION, + CommentTarget.Kind.STATISTICS); + + private static final Set UNQUALIFIED = EnumSet.of( + CommentTarget.Kind.SCHEMA, CommentTarget.Kind.ACCESS_METHOD, + CommentTarget.Kind.DATABASE, CommentTarget.Kind.EVENT_TRIGGER, + CommentTarget.Kind.EXTENSION, CommentTarget.Kind.FOREIGN_DATA_WRAPPER, + CommentTarget.Kind.LANGUAGE, CommentTarget.Kind.PUBLICATION, CommentTarget.Kind.ROLE, + CommentTarget.Kind.SERVER, CommentTarget.Kind.SUBSCRIPTION, + CommentTarget.Kind.TABLESPACE); + static Stream names() { List cases = new ArrayList<>(); - for (CommentTarget.Kind kind : List.of(CommentTarget.Kind.INDEX, CommentTarget.Kind.SCHEMA, - CommentTarget.Kind.SEQUENCE, CommentTarget.Kind.DOMAIN, CommentTarget.Kind.TYPE, - CommentTarget.Kind.MATERIALIZED_VIEW)) { + Set kinds = EnumSet.copyOf(SCHEMA_QUALIFIED); + kinds.addAll(UNQUALIFIED); + for (CommentTarget.Kind kind : kinds) { for (String name : List.of("obj", "\"a.b\"", "\"a\"\"b.c\"", "\"한.글\"", "\".\"")) { cases.add(Arguments.of(kind, name, null, name)); } - if (kind != CommentTarget.Kind.SCHEMA) { + if (SCHEMA_QUALIFIED.contains(kind)) { cases.add(Arguments.of(kind, "public.\"a.b\"", "public", "\"a.b\"")); cases.add(Arguments.of(kind, "\"s.p\".\"a.b\"", "\"s.p\"", "\"a.b\"")); } @@ -82,14 +100,27 @@ void preservesNamedTargetsAndTheirComponents(CommentTarget.Kind kind, String nam } } + static Stream signatures() { + List cases = new ArrayList<>(); + for (CommentTarget.Kind kind : List.of(CommentTarget.Kind.FUNCTION, + CommentTarget.Kind.PROCEDURE, CommentTarget.Kind.ROUTINE)) { + for (String signature : List.of("f", "f()", "f(integer)", "f(IN arg integer)", + "f(OUT result text)", "f(INOUT arg integer)", "f(VARIADIC args text[])", + "\"s.p\".\"f.x\"(IN \"arg.x\" pg_catalog.int4[])", + "f(timestamp with time zone, double precision)")) { + cases.add(Arguments.of(kind, signature)); + } + } + return cases.stream(); + } + @ParameterizedTest - @ValueSource(strings = {"f", "f()", "f(integer)", "f(IN arg integer)", - "f(OUT result text)", "f(INOUT arg integer)", "f(VARIADIC args text[])", - "\"s.p\".\"f.x\"(IN \"arg.x\" pg_catalog.int4[])", - "f(timestamp with time zone, double precision)"}) - void preservesRoutineSignaturesWithoutTreatingThemAsCalls(String signature) throws Exception { - for (Comment comment : roundTrip("COMMENT ON FUNCTION " + signature + " IS 'body'")) { - assertEquals(CommentTarget.Kind.FUNCTION, comment.getTarget().getKind()); + @MethodSource("signatures") + void preservesRoutineSignaturesWithoutTreatingThemAsCalls(CommentTarget.Kind kind, + String signature) throws Exception { + for (Comment comment : roundTrip("COMMENT ON " + kind + " " + signature + " IS 'body'")) { + assertEquals(kind, comment.getTarget().getKind()); + assertNull(comment.getTarget().getName()); RoutineReference routine = comment.getTarget().getRoutine(); assertEquals( signature.contains("(") ? signature.substring(0, signature.indexOf('(')) @@ -117,6 +148,41 @@ void exposesArgumentModeNameAndDataType() throws Exception { assertEquals("text[]", arguments.get(1).getDataType().toString()); } + @Test + void preservesAggregateSignatures() throws Exception { + RoutineReference star = aggregate("app.agg(*)"); + assertEquals("app.agg", star.getName()); + assertTrue(star.isAllArguments()); + assertNull(star.getOrderByArguments()); + + RoutineReference direct = aggregate("agg(integer, text)"); + assertFalse(direct.isAllArguments()); + assertEquals(2, direct.getArguments().size()); + assertNull(direct.getOrderByArguments()); + + RoutineReference ordered = aggregate("rank(VARIADIC \"any\" ORDER BY VARIADIC \"any\")"); + assertEquals(RoutineReference.Argument.Mode.VARIADIC, + ordered.getArguments().get(0).getMode()); + assertEquals(RoutineReference.Argument.Mode.VARIADIC, + ordered.getOrderByArguments().get(0).getMode()); + + // ORDER is not reserved, so it must not be taken for the name of a direct argument + RoutineReference orderedOnly = aggregate("mode(ORDER BY anyelement)"); + assertTrue(orderedOnly.getArguments().isEmpty()); + assertEquals("anyelement", + orderedOnly.getOrderByArguments().get(0).getDataType().toString()); + } + + private static RoutineReference aggregate(String signature) throws JSQLParserException { + List comments = roundTrip("COMMENT ON AGGREGATE " + signature + " IS NULL"); + for (Comment comment : comments) { + assertEquals(CommentTarget.Kind.AGGREGATE, comment.getTarget().getKind()); + assertEquals(signature, comment.getTarget().getRoutine().toString()); + assertTrue(new TablesNamesFinder().getTables(comment).isEmpty()); + } + return comments.get(0).getTarget().getRoutine(); + } + @ParameterizedTest @ValueSource(strings = {"t", "\"t.x\"", "public.\"t.x\"", "\"s.p\".\"t.x\""}) void distinguishesConstraintTablesFromDomains(String owner) throws Exception { @@ -146,12 +212,156 @@ void domainKeywordCanNameTheConstraintTable(String name) throws Exception { } } + static Stream relationOwnedTargets() { + List cases = new ArrayList<>(); + for (CommentTarget.Kind kind : List.of(CommentTarget.Kind.TRIGGER, + CommentTarget.Kind.POLICY, CommentTarget.Kind.RULE)) { + for (String owner : List.of("t", "\"t.x\"", "public.\"t.x\"", "\"s.p\".\"t.x\"", + "domain")) { + cases.add(Arguments.of(kind, owner)); + } + } + return cases.stream(); + } + + @ParameterizedTest + @MethodSource("relationOwnedTargets") + void namesTriggersPoliciesAndRulesWithinTheirTable(CommentTarget.Kind kind, String owner) + throws Exception { + for (Comment comment : roundTrip( + "COMMENT ON " + kind + " \"o.n\" ON " + owner + " IS 'body'")) { + CommentTarget target = comment.getTarget(); + assertEquals(kind, target.getKind()); + assertEquals("\"o.n\"", target.getName().getName()); + assertNull(target.getName().getSchemaName()); + assertFalse(target.isOnDomain()); + assertEquals(owner, target.getRelation().getFullyQualifiedName()); + assertEquals(Set.of(owner), new TablesNamesFinder().getTables(comment)); + } + } + + @ParameterizedTest + @ValueSource(strings = {"bigint AS int4", "character varying AS text", + "text[] AS app.custom_type", "timestamp with time zone AS date", + "double precision AS time without time zone"}) + void preservesCastSourceAndTargetTypes(String types) throws Exception { + for (Comment comment : roundTrip("COMMENT ON CAST (" + types + ") IS 'body'")) { + CommentTarget target = comment.getTarget(); + assertEquals(CommentTarget.Kind.CAST, target.getKind()); + assertNull(target.getName()); + assertNull(target.getRoutine()); + assertEquals(types, target.getSourceType() + " AS " + target.getTargetType()); + assertTrue(new TablesNamesFinder().getTables(comment).isEmpty()); + } + } + + @Test + void recordsProceduralLanguageSpelling() throws Exception { + for (boolean procedural : List.of(false, true)) { + String sql = "COMMENT ON " + (procedural ? "PROCEDURAL " : "") + + "LANGUAGE plpgsql IS 'body'"; + for (Comment comment : roundTrip(sql)) { + assertEquals(CommentTarget.Kind.LANGUAGE, comment.getTarget().getKind()); + assertEquals(procedural, comment.getTarget().isProcedural()); + assertEquals("plpgsql", comment.getTarget().getName().getName()); + assertEquals(sql, comment.toString()); + } + } + } + + @Test + void kindPredicatesDescribeTheReferenceShape() { + Set routines = EnumSet.noneOf(CommentTarget.Kind.class); + Set owned = EnumSet.noneOf(CommentTarget.Kind.class); + for (CommentTarget.Kind kind : CommentTarget.Kind.values()) { + if (kind.isRoutine()) { + routines.add(kind); + } + if (kind.isOwnedByRelation()) { + owned.add(kind); + } + } + assertEquals(EnumSet.of(CommentTarget.Kind.FUNCTION, CommentTarget.Kind.PROCEDURE, + CommentTarget.Kind.ROUTINE, CommentTarget.Kind.AGGREGATE), routines); + assertEquals(EnumSet.of(CommentTarget.Kind.CONSTRAINT, CommentTarget.Kind.TRIGGER, + CommentTarget.Kind.POLICY, CommentTarget.Kind.RULE), owned); + } + + @ParameterizedTest + @ValueSource(strings = {"COMMENT ON TRIGGER trg ON users IS 'x'", + "COMMENT ON POLICY p1 ON users IS 'x'", "COMMENT ON RULE r1 ON app.v IS NULL", + "COMMENT ON EXTENSION postgis IS 'x'", "COMMENT ON DATABASE mydb IS 'x'", + "COMMENT ON ROLE app_read IS 'x'", "COMMENT ON TABLESPACE pg_default IS ''", + "COMMENT ON EVENT TRIGGER et IS 'x'", "COMMENT ON PUBLICATION pub IS 'x'", + "COMMENT ON SUBSCRIPTION sub IS 'x'", "COMMENT ON SERVER srv IS 'x'", + "COMMENT ON FOREIGN DATA WRAPPER postgres_fdw IS 'x'", + "COMMENT ON FOREIGN TABLE app.ft IS 'x'", + "COMMENT ON PROCEDURAL LANGUAGE plpgsql IS 'x'", + "COMMENT ON COLLATION app.\"de-DE\" IS 'x'", "COMMENT ON ACCESS METHOD heap2 IS 'x'", + "COMMENT ON STATISTICS app.st IS 'x'", + "COMMENT ON PROCEDURE app.p(IN a integer, INOUT b text) IS 'x'", + "COMMENT ON ROUTINE r() IS 'x'", + "COMMENT ON AGGREGATE percentile_disc(float8 ORDER BY anyelement) IS 'x'", + "COMMENT ON CAST (bigint AS int4) IS 'x'"}) + void parsesAndDeparsesWithAndWithoutPostgreSqlDialect(String sql) throws Exception { + assertSqlCanBeParsedAndDeparsed(sql, false, + parser -> parser.withDialect(Dialect.POSTGRESQL)); + assertSqlCanBeParsedAndDeparsed(sql, false, null); + } + + @Test + void kindWordsRemainIdentifiers() throws Exception { + assertSqlCanBeParsedAndDeparsed("SELECT role, rule, server, extension, routine, " + + "statistics, collation, publication, subscription, language, procedural, " + + "access, method, wrapper FROM extension"); + Comment rule = roundTrip("COMMENT ON RULE rule ON rule IS NULL").get(0); + assertEquals("rule", rule.getTarget().getName().getName()); + assertEquals("rule", rule.getTarget().getRelation().getName()); + assertEquals("server", roundTrip("COMMENT ON SERVER server IS NULL").get(0).getTarget() + .getName().getName()); + assertEquals("role", roundTrip("COMMENT ON TABLE role IS NULL").get(0).getTable() + .getName()); + assertEquals("server.language", roundTrip("COMMENT ON COLUMN server.language IS NULL") + .get(0).getColumn().getFullyQualifiedName()); + } + + @Test + void rendersProgrammaticTargets() { + CommentTarget trigger = new CommentTarget(); + trigger.setKind(CommentTarget.Kind.TRIGGER); + trigger.setName(new Table("trg")); + trigger.setRelation(new Table("app", "t")); + assertEquals("COMMENT ON TRIGGER trg ON app.t IS 'body'", + new Comment().withTarget(trigger).withComment(new StringValue("body")).toString()); + + CommentTarget cast = new CommentTarget(); + cast.setKind(CommentTarget.Kind.CAST); + cast.setSourceType(new ColDataType("integer")); + cast.setTargetType(new ColDataType("text")); + assertEquals("COMMENT ON CAST (integer AS text) IS NULL", + new Comment().withTarget(cast).toString()); + + CommentTarget named = new CommentTarget(); + named.setKind(CommentTarget.Kind.ROLE); + named.setName(new Table("r")); + named.setProcedural(true); + assertEquals("ROLE r", named.toString()); + named.setKind(CommentTarget.Kind.LANGUAGE); + assertEquals("PROCEDURAL LANGUAGE r", named.toString()); + } + @ParameterizedTest @ValueSource(strings = {"INDEX", "MATERIALIZED obj", "MATERIALIZED VIEW", "FUNCTION f(*)", "FUNCTION f(integer ORDER BY integer)", "FUNCTION f(IN)", "FUNCTION f(x integer DEFAULT 1)", "CONSTRAINT ck", "CONSTRAINT ck ON", "CONSTRAINT public.ck ON t", + "TRIGGER trg", "TRIGGER trg ON", "TRIGGER app.trg ON t", "TRIGGER trg ON DOMAIN d", + "POLICY p", "RULE r ON DOMAIN d", "PROCEDURE p(*)", + "ROUTINE r(integer ORDER BY integer)", "FUNCTION f(ORDER BY integer)", + "AGGREGATE agg", "CAST (integer)", "CAST integer AS text", "CAST (integer AS text", + "FOREIGN ft", "FOREIGN DATA fdw", "EVENT et", "ACCESS heap", "ACCESS METHOD", + "PROCEDURAL plpgsql", "LANGUAGE", "ROLE", "UNKNOWN obj"}) void rejectsIncompleteOrInvalidTargets(String target) { assertThrows(JSQLParserException.class, () -> parse("COMMENT ON " + target + " IS NULL")); @@ -160,12 +370,17 @@ void rejectsIncompleteOrInvalidTargets(String target) { @Test void findsOnlyRelationsExplicitlyNamedByComments() throws Exception { for (String target : List.of("INDEX idx", "SCHEMA app", "SEQUENCE seq", "DOMAIN d", - "TYPE ty", "FUNCTION f(int)")) { + "TYPE ty", "FUNCTION f(int)", "PROCEDURE app.p(int)", "ROUTINE r", + "AGGREGATE a(*)", "CAST (integer AS text)", "COLLATION app.c", + "STATISTICS app.st", "ACCESS METHOD am", "DATABASE d", "EVENT TRIGGER et", + "EXTENSION e", "FOREIGN DATA WRAPPER w", "LANGUAGE l", "PUBLICATION p", "ROLE r", + "SERVER s", "SUBSCRIPTION s", "TABLESPACE ts")) { assertTrue(new TablesNamesFinder().getTables(parse("COMMENT ON " + target + " IS NULL")) .isEmpty()); } for (String target : List.of("TABLE app.t", "COLUMN app.t.c", "VIEW app.t", - "MATERIALIZED VIEW app.t", "CONSTRAINT ck ON app.t")) { + "MATERIALIZED VIEW app.t", "CONSTRAINT ck ON app.t", "FOREIGN TABLE app.t", + "TRIGGER trg ON app.t", "POLICY p ON app.t", "RULE r ON app.t")) { assertEquals(Set.of("app.t"), new TablesNamesFinder().getTables(parse("COMMENT ON " + target + " IS NULL"))); } @@ -223,9 +438,11 @@ public Void visit(StringValue value, S context) { assertEquals(List.of("body"), values); } - @Test - void customDeparsersCanRewriteRelationsAndLiterals() throws Exception { - Comment comment = parse("COMMENT ON CONSTRAINT ck ON app.t IS 'body'"); + @ParameterizedTest + @ValueSource(strings = {"CONSTRAINT ck ON", "TRIGGER trg ON", "POLICY p ON", "RULE r ON", + "FOREIGN TABLE", "MATERIALIZED VIEW"}) + void customDeparsersCanRewriteRelationsAndLiterals(String target) throws Exception { + Comment comment = parse("COMMENT ON " + target + " app.t IS 'body'"); StringBuilder builder = new StringBuilder(); ExpressionDeParser expressions = new ExpressionDeParser() { @Override @@ -242,8 +459,8 @@ public StringBuilder visit(Table table, S context) { } }; comment.accept(new StatementDeParser(expressions, selects, builder), "context"); - assertEquals("COMMENT ON CONSTRAINT ck ON app.new_t IS 'changed'", builder.toString()); - assertEquals("COMMENT ON CONSTRAINT ck ON app.t IS 'body'", comment.toString()); + assertEquals("COMMENT ON " + target + " app.new_t IS 'changed'", builder.toString()); + assertEquals("COMMENT ON " + target + " app.t IS 'body'", comment.toString()); } private static Comment parse(String sql) throws JSQLParserException { diff --git a/src/test/java/net/sf/jsqlparser/util/validation/validator/CommentValidatorTest.java b/src/test/java/net/sf/jsqlparser/util/validation/validator/CommentValidatorTest.java index 276e416e9..fe26d321e 100644 --- a/src/test/java/net/sf/jsqlparser/util/validation/validator/CommentValidatorTest.java +++ b/src/test/java/net/sf/jsqlparser/util/validation/validator/CommentValidatorTest.java @@ -9,8 +9,15 @@ */ package net.sf.jsqlparser.util.validation.validator; +import static org.junit.jupiter.api.Assertions.assertEquals; +import java.util.EnumSet; +import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; +import net.sf.jsqlparser.parser.CCJSqlParserUtil; import net.sf.jsqlparser.parser.feature.Feature; +import net.sf.jsqlparser.statement.comment.Comment; +import net.sf.jsqlparser.statement.comment.CommentTarget; import net.sf.jsqlparser.util.validation.ValidationTestAsserts; import net.sf.jsqlparser.util.validation.feature.DatabaseType; import net.sf.jsqlparser.util.validation.feature.FeaturesAllowed; @@ -29,7 +36,39 @@ static Stream targets() { Arguments.of("MATERIALIZED VIEW mv", Feature.commentOnMaterializedView), Arguments.of("FUNCTION f(IN x integer)", Feature.commentOnFunction), Arguments.of("CONSTRAINT ck ON t", Feature.commentOnConstraint), - Arguments.of("CONSTRAINT ck ON DOMAIN d", Feature.commentOnConstraint)); + Arguments.of("CONSTRAINT ck ON DOMAIN d", Feature.commentOnConstraint), + Arguments.of("TRIGGER trg ON t", Feature.commentOnTrigger), + Arguments.of("POLICY p ON t", Feature.commentOnPolicy), + Arguments.of("RULE r ON v", Feature.commentOnRule), + Arguments.of("PROCEDURE p(integer)", Feature.commentOnProcedure), + Arguments.of("ROUTINE r", Feature.commentOnRoutine), + Arguments.of("AGGREGATE a(*)", Feature.commentOnAggregate), + Arguments.of("CAST (integer AS text)", Feature.commentOnCast), + Arguments.of("ACCESS METHOD am", Feature.commentOnAccessMethod), + Arguments.of("COLLATION c", Feature.commentOnCollation), + Arguments.of("DATABASE d", Feature.commentOnDatabase), + Arguments.of("EVENT TRIGGER et", Feature.commentOnEventTrigger), + Arguments.of("EXTENSION e", Feature.commentOnExtension), + Arguments.of("FOREIGN DATA WRAPPER w", Feature.commentOnForeignDataWrapper), + Arguments.of("FOREIGN TABLE ft", Feature.commentOnForeignTable), + Arguments.of("PROCEDURAL LANGUAGE l", Feature.commentOnLanguage), + Arguments.of("PUBLICATION p", Feature.commentOnPublication), + Arguments.of("ROLE r", Feature.commentOnRole), + Arguments.of("SERVER s", Feature.commentOnServer), + Arguments.of("STATISTICS st", Feature.commentOnStatistics), + Arguments.of("SUBSCRIPTION s", Feature.commentOnSubscription), + Arguments.of("TABLESPACE ts", Feature.commentOnTablespace)); + } + + @Test + void coversEveryTargetKind() throws Exception { + Set kinds = EnumSet.noneOf(CommentTarget.Kind.class); + for (Arguments arguments : targets().collect(Collectors.toList())) { + Comment comment = (Comment) CCJSqlParserUtil + .parse("COMMENT ON " + arguments.get()[0] + " IS NULL"); + kinds.add(comment.getTarget().getKind()); + } + assertEquals(EnumSet.allOf(CommentTarget.Kind.class), kinds); } @ParameterizedTest