Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1042 +/- ##
============================================
- Coverage 91.92% 91.76% -0.16%
- Complexity 283 289 +6
============================================
Files 27 27
Lines 1015 1044 +29
Branches 86 91 +5
============================================
+ Hits 933 958 +25
- Misses 54 56 +2
- Partials 28 30 +2 🚀 New features to boost your workflow:
|
f56d22d to
69071d3
Compare
69071d3 to
4924085
Compare
keoliva
changed the title
4924085 to
7d0ebb3
Compare
|
New approach: A simple boolean flag that controls both the auto-generated schema (timestamp-micros vs timestamp-millis) and the value conversion (getNanos() vs getTime()). No external schema file needed that has to be kept up-to-date and the fields ordered in a particular order, because schema and data both come from the JDBC ResultSet. |
| final List<String> preCommand, | ||
| final String arrayMode, | ||
| final Boolean nullableArrayItems, | ||
| final Boolean useTimestampMicros) { |
There was a problem hiding this comment.
Shouldn't this just read the option with a default value of false?
There was a problem hiding this comment.
When you say ‘read the option,’ which object would we read it from? JdbcExportPipelineOptions comes to mind, but it isn’t available in this method.
There was a problem hiding this comment.
I meant rather than providing an override; all callers should provide a value for this
|
|
||
| private Schema createAvroSchemaForSingleField( | ||
| final ResultSet resultSet, final boolean useLogicalTypes, | ||
| final boolean useTimestampMicros) throws SQLException { |
There was a problem hiding this comment.
Again, shouldn't this be reading the default value of the opt instead of having an override?
There was a problem hiding this comment.
Updated this one too. All callers now explicitly provide a value.
There was a problem hiding this comment.
…nal schema Replaces the previous approach of reading timestamp-micros from an explicit --avroSchemaFilePath schema. The old approach required hand-maintained .avsc files whose field order had to exactly match the Postgres column order — a brittle contract that caused production failures when ALTER TABLE columns were placed in "logical" positions in the schema instead of their actual Postgres ordinal positions (Avro binary is positional, so any mismatch corrupts the data stream). The new --useTimestampMicros flag is simpler and safer: - Schema generation: emits timestamp-micros (not timestamp-millis) in the auto-generated Avro schema when --useAvroLogicalTypes is also set - Value conversion: reads Timestamp.getNanos() to preserve microsecond precision instead of truncating via Timestamp.getTime() - Field order is always correct because both schema and data come from the same JDBC ResultSet — no external schema file to keep in sync Usage: --useAvroLogicalTypes --useTimestampMicros This is fully backwards-compatible: without --useTimestampMicros, all behavior is identical to before. Co-Authored-By: Claude <noreply@anthropic.com>
7d0ebb3 to
7d68033
Compare
Co-authored-by: OpenAI Codex <codex@openai.com>
| case DATE: | ||
| case TIME: | ||
| case TIME_WITH_TIMEZONE: | ||
| if (useTimestampMicros) { |
There was a problem hiding this comment.
useTimestampMicros without useAvroLogicalTypes produces micros values with no schema annotation. AFAIK data processing tools interpret Avro long without logical type annotation as millis, and this is an established convention in DBeam. Maybe we should not allow using useTimestampMicros without useAvroLogicalTypes?
Co-authored-by: OpenAI Codex <codex@openai.com>
4 participants
Why
PostgreSQL stores
TIMESTAMPTZvalues at microsecond precision, but dbeam currently converts them throughTimestamp.getTime(), truncating the sub-millisecond digits.What
This adds
--useTimestampMicros. When it is combined with--useAvroLogicalTypes, dbeam:timestamp-microsin the schema generated from the JDBCResultSet; andTimestamp.getNanos()so the microsecond component is preserved.Schema and data now come from the same
ResultSet, so no external schema file is needed and field order remains aligned.Without
--useTimestampMicros, behavior is unchanged.e2e results
before & after
