[Fix][Connector-V2] ArrowToSeatunnelRowReader convertSeatunnelRowValue add handle Second TIMESTAMP type #9393
Conversation
Signed-off-by: dyp12 <[email protected]>
|
cc @liugddx |
Hisoka-X
changed the title
Hisoka-X
changed the title
| .atZone(ZoneId.systemDefault()) | ||
| .toLocalDateTime(); | ||
| // this TIMESTAMP value may be SECOND not milliseconds | ||
| if (((Long) fieldValue).toString().length() == 10) { |
There was a problem hiding this comment.
We should get the precision from the type rather than judging directly by the size of the value, which may be wrong.
There was a problem hiding this comment.
We should get the precision from the type rather than judging directly by the size of the value, which may be wrong.
sorry, i fix it, using Types.MinorType to know
|
Please add test case |
Signed-off-by: dyp12 <[email protected]>
Signed-off-by: dyp12 <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This pull request implements a fix for the ArrowToSeatunnelRowReader, adding support for handling TIMESTAMP values with SECOND precision (with and without timezone). The main changes include updating data type mappings in test helpers, adjusting test cases to verify new timestamp behaviors, and adding a new TimeStampSecConverter.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| SeaTunnelDataTypeHolder.java | Added cases for "timestampSec" and "timestampSecTz" mapping to LocalDateTime. |
| ArrowToSeatunnelRowReaderTest.java | Extended tests to cover SECOND precision timestamp vectors. |
| META-INF/services/.../Converter | Registered the new TimeStampSecConverter. |
| ArrowToSeatunnelRowReader.java | Updated row value conversion logic to handle second timestamps. |
| TimeStampSecConverter.java | Introduced a converter to transform second precision Arrow timestamp values. |
Comments suppressed due to low confidence (2)
seatunnel-connectors-v2/connector-common/src/main/java/org/apache/seatunnel/connectors/seatunnel/common/source/arrow/reader/ArrowToSeatunnelRowReader.java:208
- Consider adding more detailed comments to explain why TIMESTAMP values are treated as seconds rather than milliseconds in this block, to improve code clarity and maintainability.
if (Types.MinorType.TIMESTAMPSEC == minorType || Types.MinorType.TIMESTAMPSECTZ == minorType) {
seatunnel-connectors-v2/connector-common/src/main/java/org/apache/seatunnel/connectors/seatunnel/common/source/arrow/converter/TimeStampSecConverter.java:33
- Add a comment clarifying that this converter assumes the input timestamp is in UTC before converting it to the system default timezone.
LocalDateTime localDateTime = fieldVector.getObject(rowIndex);
thank you for your help |
…e add handle Second TIMESTAMP type (apache#9393) Signed-off-by: dyp12 <[email protected]> Co-authored-by: dyp12 <[email protected]>
3 participants
Purpose of this pull request
close #8352
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide