Skip to content

Delimit identifiers in the generated CREATE PUBLICATION statement for the Postgres RDS source - #7078

Open
ryan-gray-chipply wants to merge 1 commit into
opensearch-project:mainfrom
ryan-gray-chipply:fix/postgres-publication-quoted-identifiers
Open

Delimit identifiers in the generated CREATE PUBLICATION statement for the Postgres RDS source#7078
ryan-gray-chipply wants to merge 1 commit into
opensearch-project:mainfrom
ryan-gray-chipply:fix/postgres-publication-quoted-identifiers

Conversation

@ryan-gray-chipply

Copy link
Copy Markdown

Description

The RDS PostgreSQL source builds CREATE PUBLICATION … FOR TABLE … by concatenating schema-discovered database.schema.table names without delimiting them. PostgreSQL folds undelimited identifiers to lower case and rejects characters such as hyphens, so any name that requires a delimited identifier makes publication creation fail and aborts pipeline initialization:

CREATE PUBLICATION dp_pub FOR TABLE My-Db-1.dbo.MyTable;      -- ERROR: syntax error at or near "-"
CREATE PUBLICATION dp_pub FOR TABLE mydb.dbo.MyTable;         -- ERROR: relation "dbo.mytable" does not exist

This change adds quoteFullTableName / quoteIdentifier helpers and delimits each part of the name when building the statement:

CREATE PUBLICATION dp_pub FOR TABLE "My-Db-1"."dbo"."MyTable";

Both forms above were run against PostgreSQL 17.10 to confirm the unquoted statements fail and the delimited one succeeds.

Delimiting is safe rather than merely tolerable here: the names originate from JDBC metadata (TABLE_SCHEM / TABLE_NAME) via PostgresSchemaManager.getTableNames, so they already carry the true stored case. For names that need no quoting the change is a semantic no-op, covered by a regression test.

Three things this deliberately does not change:

  • The publication name is left undelimited, in both CREATE PUBLICATION and DROP PUBLICATION IF EXISTS. It is generated by Data Prepper and recorded in the pipeline state store; delimiting it would change how a publication created by an earlier version gets dropped after an upgrade — a mixed-case pipeline name would fold on create but not on drop, leaking the publication. Keeping both sides undelimited keeps create/drop symmetric.
  • The database qualifier is kept. It is redundant — PostgreSQL only accepts a catalog qualifier equal to the current database — but dropping it is a separate behavior change and does not belong in a bug fix.
  • MySQL is untouched. MySqlSchemaManager has its own (backtick) quoting considerations; a separate concern.

One pre-existing limitation is worth flagging but is out of scope: several methods in this class split a fully qualified name on "\\.", which misparses identifiers containing a literal dot. Noted in the issue as a follow-up.

Tests

Updated (the two existing assertions hard-coded the undelimited SQL):

  • test_createLogicalReplicationSlot_creates_slot_if_not_exists
  • test_createLogicalReplicationSlot_skip_creation_if_slot_exists

Added:

Test Covers
test_createLogicalReplicationSlot_delimits_identifiers_that_require_quoting end-to-end statement for a hyphenated database + mixed-case table, two tables, separator preserved
test_quoteFullTableName_delimits_every_part_and_preserves_case hyphenated database; mixed case preserved, not folded
test_quoteFullTableName_when_name_needs_no_quoting_then_only_adds_delimiters regression guard — no-op for all-lowercase names
test_quoteIdentifier_escapes_embedded_double_quote " doubled to ""

test_deleteLogicalReplicationSlot_success is unchanged and still passes, which is what confirms the publication-name boundary described above.

:data-prepper-plugins:rds-source:check passes locally (spotless, checkstyle, and the module's tests — 20/20).

Issues Resolved

Resolves #7077

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • No documentation change needed — this is a bug fix that makes the source work with names the docs never excluded; the limitation was never documented.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…source

The RDS PostgreSQL source concatenated schema-discovered
database.schema.table names into CREATE PUBLICATION without delimiting
them. PostgreSQL folds undelimited identifiers to lower case and rejects
characters such as hyphens, so any name requiring a delimited identifier
failed publication creation and aborted pipeline initialization.

Delimit each part of the name when building the statement. The names come
from JDBC metadata and already carry the true stored case, so quoting
them makes the lookup correct rather than merely legal. The generated
publication name is intentionally left undelimited so that create and
drop stay symmetric across an upgrade.

Signed-off-by: Ryan Gray <ryan@chipply.com>
@ryan-gray-chipply

Copy link
Copy Markdown
Author

Could this be considered for a backport 2.x label? The bug makes the Postgres RDS source unusable for any database, schema, or table whose name needs a delimited identifier, and reaching a patch release would let downstream managed distributions pick it up sooner.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Naive Split on Dot

quoteFullTableName splits fullTableName on . without accounting for identifiers that themselves contain dots. If a database, schema, or table name legitimately contains a . character (allowed in PostgreSQL when quoted), the split will produce more than three parts and the resulting quoted name will incorrectly break the identifier into multiple pieces (e.g., db.name.schema.table becomes four quoted parts instead of three). This is a narrow scenario since JDBC metadata rarely returns such names, but the helper's contract of database.schema.table is implicit and fragile.

static String quoteFullTableName(final String fullTableName) {
    final String[] splits = fullTableName.split("\\.");
    final StringBuilder quotedName = new StringBuilder();
    for (int i = 0; i < splits.length; i++) {
        if (i > 0) {
            quotedName.append(".");
        }
        quotedName.append(quoteIdentifier(splits[i]));
    }
    return quotedName.toString();
}

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid emitting invalid three-part table names

Splitting the fully qualified name on . will incorrectly break identifiers that
legitimately contain a dot (e.g. "my.schema"), and it will also silently mishandle
names that already contain literal dots between unquoted parts. Since PostgreSQL
CREATE PUBLICATION only accepts schema.table (not database.schema.table), consider
dropping the database prefix and only quoting the remaining two parts, or validating
the expected part count to avoid producing malformed DDL.

data-prepper-plugins/rds-source/src/main/java/org/opensearch/dataprepper/plugins/source/rds/schema/PostgresSchemaManager.java [306-316]

 static String quoteFullTableName(final String fullTableName) {
     final String[] splits = fullTableName.split("\\.");
+    if (splits.length < 2) {
+        throw new IllegalArgumentException("Expected fully qualified table name in database.schema.table form: " + fullTableName);
+    }
+    // Skip the database prefix; PostgreSQL CREATE PUBLICATION only accepts schema-qualified table names.
+    final int startIndex = splits.length == 3 ? 1 : 0;
     final StringBuilder quotedName = new StringBuilder();
-    for (int i = 0; i < splits.length; i++) {
-        if (i > 0) {
+    for (int i = startIndex; i < splits.length; i++) {
+        if (i > startIndex) {
             quotedName.append(".");
         }
         quotedName.append(quoteIdentifier(splits[i]));
     }
     return quotedName.toString();
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion raises a valid concern that PostgreSQL CREATE PUBLICATION requires schema-qualified names (not database-qualified), so emitting three-part names like "My-Db-1"."dbo"."MyTable" could produce invalid DDL. However, the test cases in the PR assert the three-part form as expected behavior, so this may reflect an existing convention; the impact is moderate and requires verification.

Low

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.

[BUG] RDS PostgreSQL source: CREATE PUBLICATION uses unquoted identifiers, failing on hyphenated or mixed-case database/schema/table names

1 participant