feat: Support PostgreSQL export for export-metadata#19698
Conversation
b69c7a6 to
baa141e
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…r-only permissions
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 2 |
| P2 | 1 |
| P3 | 0 |
| Total | 3 |
Reviewed 7 of 7 changed files.
Found three export-migration risks: buffered PostgreSQL reads, dropped current-schema segment columns, and lost CSV escaping during rewrites.
This is an automated review by Codex GPT-5.6-Sol
| { | ||
| retryWithHandle( | ||
| (HandleCallback<Void>) handle -> { | ||
| try (Statement stmt = handle.getConnection().createStatement(); |
There was a problem hiding this comment.
[P1] Stream PostgreSQL results instead of buffering the table
PostgreSQL JDBC buffers the complete ResultSet by default. This plain Statement runs on an auto-commit connection and never sets a fetch size; PostgreSQL cursor-based fetching requires autoCommit=false and a positive fetch size. Exporting a production segments table can therefore exhaust heap before rows are written. Run the query in a transaction and apply getStreamingFetchSize().
| final String exportTableName = isDerby() ? StringUtils.toUpperCase(tableName) : tableName; | ||
| dbConnector.exportTable( | ||
| StringUtils.toUpperCase(tableName), | ||
| exportTableName, |
There was a problem hiding this comment.
[P1] Preserve current segment columns in the importable CSV
The generic raw export preserves all columns, but run() immediately passes the file through rewriteSegmentsExport, which emits only columns 0 through 8. Current segment tables also contain required used_status_last_updated and additional fingerprint/upgrade columns. A PostgreSQL migration therefore loses those values, and the documented COPY into a freshly created current table fails because used_status_last_updated is NOT NULL without a default. The new test covers only the raw file; the final CSV and import commands need to preserve the current schema.
| } else { | ||
| final String val = rs.getString(i); | ||
| if (val != null) { | ||
| if (val.contains(",") || val.contains("\"") || val.contains("\n") || val.contains("\r")) { |
There was a problem hiding this comment.
[P2] Preserve CSV escaping through the rewrite stage
Although the raw writer correctly quotes commas and double quotes, each ExportMetadata rewrite parses those fields and concatenates them into the final CSV without re-escaping. Legal datasource and metadata identifiers may contain these characters, so the final importable file gains extra columns or malformed quoting even though the raw file is valid. Use a CSV writer or shared escaping routine in the rewrite stage and add an end-to-end test.
3 participants
Description
The
export-metadatatool currently only supports exporting from Derby metadata stores. This PR adds support for exporting from PostgreSQL by implementing a generic JDBC-basedexportTableinSQLMetadataConnector, which PostgreSQL (and any future connector) inherits automatically.Added generic JDBC export in
SQLMetadataConnectorImplemented
exportTableWithJdbc, aprotectedmethod that exports any table to CSV using standard JDBC. Binary/BLOB columns (including PostgreSQLBYTEA) are hex-encoded, booleans are written astrue/falsestrings, and string values containing commas, quotes,\n, or\rare properly CSV-escaped per RFC 4180. The baseexportTabledelegates to this method, whileDerbyConnectorcontinues to override it with Derby's nativeSYSCS_EXPORT_TABLE.Updated
ExportMetadatafor PostgreSQL compatibilityjdbc:derbyURI prefix), since PostgreSQL uses lowercase table names.@Commanddescription to mention PostgreSQL support.Added unit tests
Four new tests in
SQLMetadataConnectorTestexercise the generic JDBC export path viaTestDerbyConnector.exportTableGeneric():testExportTable— verifies hex-encoded BLOBs andtrue/falseboolean stringstestExportTableWithSpecialCharacters— verifies CSV quoting/escaping for commas, double quotes, and plain valuestestExportTableWithNullValues— verifies NULL columns produce empty CSV fieldstestExportTablePreservesAllColumns— verifies all columns (including nullable trailing columns likeused_status_last_updated) are exportedUpdated documentation
export-metadata.md— removed the Derby-only limitation, added a "PostgreSQL" section under "Running the tool" with the required-Ddruid.extensions.loadListand-Ddruid.metadata.storage.typeflags, clarified_raw.csvdescriptionmetadata-migration.md— updated intro and export tool reference to include PostgreSQLdeep-storage-migration.md— updated export tool reference, added note about no running processes needed when migrating from PostgreSQLRelease note
The
export-metadatatool now supports exporting from PostgreSQL metadata stores in addition to Derby. When exporting from PostgreSQL, pass-Ddruid.extensions.loadList='["postgresql-metadata-storage"]' -Ddruid.metadata.storage.type=postgresqlon the command line along with the appropriate--connectURI.Key changed/added classes in this PR
SQLMetadataConnector— addedexportTableandexportTableWithJdbcfor generic JDBC CSV exportExportMetadata— addedisDerby()helper, conditional table name casingTestDerbyConnector— addedexportTableGeneric()to test the generic JDBC pathSQLMetadataConnectorTest— added 4 export testsThis PR has: