feat: Support PostgreSQL export for export-metadata - #19698
Conversation
b69c7a6 to
baa141e
Compare
…r-only permissions
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().
There was a problem hiding this comment.
Confirmed: retryTransaction gives PostgreSQL an auto-commit-disabled connection, and getStreamingFetchSize() is applied before query execution, enabling cursor-based fetching. Reviewed 8 of 8 changed files.
| 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.
There was a problem hiding this comment.
Thanks; the rewrite now preserves trailing values, but the documented PostgreSQL COPY druid_segments(...) command still names only the original nine columns. Current exports contain 12–14 fields, so COPY fails with extra data; upgraded source tables can also have added columns in a different physical order. Please export segments in an explicit stable column order and update the import column lists accordingly. Reviewed 8 of 8 changed files.
| } 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.
There was a problem hiding this comment.
The comma/quote round-trip is fixed, but multiline fields are still split because each rewrite uses BufferedReader.readLine() plus CSVParser.parseLine() on one physical line. exportTableWithJdbc writes quoted newline or carriage-return values as CSV records spanning lines, so the rewrite misparses them before csvEscapeField runs. Please use a record-aware CSVReader/readNext path and cover this end to end. Reviewed 8 of 8 changed files.
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 1 |
| P3 | 0 |
| Total | 2 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 1 |
| P3 | 0 |
| Total | 2 |
Reviewed 8 of 8 changed files. PostgreSQL streaming is fixed, but segment import compatibility and CSV backslash preservation remain broken.
This is an automated review by Codex GPT-5.6-Sol
|
|
||
| // Preserve any additional columns after payload (e.g. used_status_last_updated, | ||
| // indexing_state_fingerprint, upgraded_from_segment_id, schema_fingerprint, num_rows) | ||
| for (int i = 9; i < parsed.length; i++) { |
There was a problem hiding this comment.
[P1] Update imports for preserved segment columns
Current segment tables emit at least 12 fields, and this loop now retains all of them, but the documented PostgreSQL and MySQL import commands still declare only the original nine columns. PostgreSQL COPY rejects every such row as extra data, while MySQL truncates or ignores trailing fields with warnings. Emit a deterministic schema-compatible order and update import commands to include all exported columns.
| } 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 backslashes through CSV rewrites
The writer emits RFC 4180 data and leaves backslashes unchanged, but ExportMetadata reads it with default OpenCSV CSVParser, where backslash is the escape character. Consequently, a valid identifier such as foo\bar is parsed as foobar before csvEscapeField runs; Druid's ID validation explicitly permits backslashes. Use an RFC 4180 reader/parser or configure a null escape character, and add a round-trip test.
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 1 |
| P3 | 0 |
| Total | 2 |
Reviewed 9 of 9 changed files. Stable segment ordering and RFC CSV parsing fix the prior issues, but legacy schemas still conflict with the documented import list and mixed-case PostgreSQL bases fail column discovery.
This is an automated review by Codex GPT-5.6-Sol
|
|
||
| These example import commands expect `/tmp/csv` and its contents to be accessible from the server. For other options, such as importing from the client filesystem, please refer to the database's documentation. | ||
|
|
||
| The segments table is exported in a fixed column order, independent of the physical column order of the source table: `id`, `dataSource`, `created_date`, `start`, `end`, `partitioned`, `version`, `used`, `payload`, `used_status_last_updated`, `indexing_state_fingerprint`, `upgraded_from_segment_id`, followed by `schema_fingerprint` and `num_rows` if the source table has them. Add `schema_fingerprint,num_rows` to the end of the segments column list in the import commands below if those columns are present. |
There was a problem hiding this comment.
[P1] Keep import lists aligned with older segment schemas
orderSegmentsColumns skips every absent column, and the new test explicitly supports legacy nine-column tables, but the documented PostgreSQL COPY always expects the three later columns. A nine-field export therefore fails with missing data. The instructions or generated output must identify every optional post-payload column and omit absent ones from the import list and FORCE_NULL.
| try (ResultSet rs = dbMetaData.getColumns( | ||
| null, | ||
| escapeMetaDataSearchString(dbMetaData, conn.getSchema()), | ||
| escapeMetaDataSearchString(dbMetaData, tableName), |
There was a problem hiding this comment.
[P2] Normalize PostgreSQL identifiers before metadata lookup
PostgreSQL folds unquoted identifiers to lowercase, while DatabaseMetaData.getColumns uses a case-sensitive LIKE. With a valid mixed-case --base, tableExists succeeds via ILIKE and ordinary SQL resolves the lowercase table, but this lookup returns no columns, causing segment export to abort. Normalize according to JDBC identifier rules or resolve the actual table name first.
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 1 |
| P3 | 0 |
| Total | 2 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 1 |
| P3 | 0 |
| Total | 2 |
Reviewed 9 of 9 changed files. Mixed-case PostgreSQL identifiers are now handled, but legacy Derby imports and configured PostgreSQL schema discovery remain incorrect.
This is an automated review by Codex GPT-5.6-Sol
|
|
||
| These example import commands expect `/tmp/csv` and its contents to be accessible from the server. For other options, such as importing from the client filesystem, please refer to the database's documentation. | ||
|
|
||
| The segments table is exported in a fixed column order, independent of the physical column order of the source table: `id`, `dataSource`, `created_date`, `start`, `end`, `partitioned`, `version`, `used`, `payload`, followed by whichever of the optional columns `used_status_last_updated`, `indexing_state_fingerprint`, `upgraded_from_segment_id`, `schema_fingerprint`, and `num_rows` exist in the source table, in that order. Adjust the segments column list in the import commands below to contain exactly the columns of the source table: omit any optional column the source table does not have (segments tables from older Druid versions may have only the first nine columns), and add `schema_fingerprint,num_rows` at the end if the source table has them. Apply the same adjustment to the columns declared with `FORCE_NULL` in the PostgreSQL command. |
There was a problem hiding this comment.
[P1] Provide a column-mapped Derby import for legacy exports
The new guidance says to omit optional columns from the import list, but the Derby example uses SYSCS_IMPORT_TABLE, which has no column list and imports every destination column in physical order. A nine-column legacy export therefore still cannot be loaded into a current 12/14-column Derby segments table, and the documented adjustment cannot be performed. Use SYSCS_IMPORT_DATA with an explicit mapping, or provide another working legacy-schema procedure.
| final DatabaseMetaData dbMetaData = conn.getMetaData(); | ||
| try (ResultSet rs = dbMetaData.getColumns( | ||
| null, | ||
| escapeMetaDataSearchString(dbMetaData, conn.getSchema()), |
There was a problem hiding this comment.
[P2] Look up columns in the configured PostgreSQL schema
PostgreSQLConnector.tableExists scopes the relation to druid.metadata.postgres.dbTableSchema, but getTableColumns scopes DatabaseMetaData.getColumns to Connection.getSchema(). These can differ, for example when a role-named schema precedes public in search_path while the Druid tables remain in public, so tableExists succeeds but column discovery returns empty and exportSegmentsTable aborts. Use the connector's configured table schema or resolve the actual schema containing the unqualified relation.
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.Fixed streaming, column preservation, and CSV escaping in export
SQLMetadataConnector.exportTableWithJdbc— switched fromretryWithHandle(auto-commit) toretryTransactionso the connection runs withautoCommit=false, and appliedgetStreamingFetchSize()to theStatement. This enables PostgreSQL cursor-based streaming instead of buffering the entireResultSetin memory.ExportMetadata.rewriteSegmentsExport— the rewrite stage previously hardcoded columns 0–8, droppingused_status_last_updated,indexing_state_fingerprint,upgraded_from_segment_id, and optional schema columns. Added a loop to pass through all columns afterpayload.ExportMetadatarewrite methods — all five rewrite methods (rewriteDatasourceExport,rewriteRulesExport,rewriteConfigExport,rewriteSupervisorExport,rewriteSegmentsExport) now re-escape non-payload fields via a newcsvEscapeField()helper (RFC 4180). Previously, fields containing commas or double quotes were parsed correctly by opencsv but written back without quoting, producing malformed output.ExportMetadataTest— added 10 tests coveringcsvEscapeField, segments rewrite with all columns, special characters round-trip, and backward compatibility with 9-column tables.Deterministic segments column order and updated import commands
SQLMetadataConnector— addedexportTable(tableName, outputPath, columns)andgetTableColumns(tableName). When a column list is given, the export query selects those columns in that order, quoting each identifier with the database's identifier quote string so reserved words such asendwork.DerbyConnectorusesSYSCS_EXPORT_QUERYin that case.ExportMetadata— the segments table is now exported with a canonical column order (id, dataSource, created_date, start, end, partitioned, version, used, payload, used_status_last_updated, indexing_state_fingerprint, upgraded_from_segment_id[, schema_fingerprint, num_rows]), instead of relying on the physical column order, which depends on the order in whichALTER TABLEadded the newer columns. Unknown columns are appended at the end.export-metadata.md— documented the exported segments column order and updated the MySQLLOAD DATAand PostgreSQLCOPYcommands to list all exported segments columns.Preserved backslashes through the CSV rewrite
ExportMetadata— the rewrite stage read the intermediate CSV with the default OpenCSVCSVParser, which treats backslash as an escape character and silently dropped it from values such as segment ids and datasource names (backslashes are permitted by Druid's id validation). The rewrite now usesRFC4180Parser, matching the RFC 4180 output written by the export stage.ExportMetadataTest— addedtestRewriteSegmentsExport_preservesBackslashes, and existing assertions now parse the output withRFC4180Parser.Record-aware CSV parsing in the rewrite stage
ExportMetadata— all five rewrite methods read one physical line at a time, so a quoted value containing a newline or carriage return (which the export stage writes as a record spanning several lines) was misparsed. They now use a sharedopenCsvReader()helper that builds aCSVReaderwith anRFC4180ParserandwithKeepCarriageReturn(true), and iterate withreadNext()so multi-line records are handled as single records.ExportMetadataTest— addedtestExportAndRewriteSegments_withMultilineFields, an end-to-end test that inserts values containing newlines, carriage returns, and commas into a Derby segments table, exports it through the generic JDBC path, runs the rewrite, and verifies every record and field round-trips.Review fixes
SQLMetadataConnector.getTableColumns— the lookup passed the raw table name as aDatabaseMetaDatasearch pattern with a null schema, so_acted as a wildcard and columns from same-named tables in other schemas could leak in. It is now scoped toConnection.getSchema()(the schema an unqualified table name resolves to, consistent withPostgreSQLConnector.tableExists) and both names are escaped withgetSearchStringEscape().SQLMetadataConnector.makeExportSelectList— doubles any identifier quote character inside a column name.ExportMetadata.exportSegmentsTable— fails with anISEinstead of silently falling back toSELECT *(and thus an unstable column order) when the column list cannot be read.ExportMetadata.readRecord— all five rewrites now validate the field count of each record and fail with the row number, file name, and expected arity instead of throwingArrayIndexOutOfBoundsExceptionon a malformed raw CSV.export-metadata.md— the PostgreSQL segments import usesFORCE_NULLfor the nullable columns, since NULLs are exported as empty fields whichCOPYwould otherwise import as empty strings (and reject outright for non-string columns).SQLMetadataConnectorTest— addedtestExportTableWithDerbyNativeExport, coveringDerbyConnector's nativeSYSCS_EXPORT_QUERYpath with an explicit column list, a reserved-word column, and a BLOB payload (written as lowercase hex).ExportMetadataTest— addedtestRewriteSegmentsExport_failsOnTruncatedRow.Key changed/added classes in this PR
SQLMetadataConnector— addedexportTableandexportTableWithJdbcfor generic JDBC CSV export; switched export to transactional streamingExportMetadata— addedisDerby()helper, conditional table name casing,csvEscapeField(), all-column preservation in segments rewriteTestDerbyConnector— addedexportTableGeneric()to test the generic JDBC pathSQLMetadataConnectorTest— added 4 export testsExportMetadataTest— added 10 tests for CSV escaping and segments rewriteThis PR has: