DX-100503: new arrow type and vector TimestampWithPrecision #83
Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or See also: |
github-actions
bot
added
Component: C++
Component: Python
Component: R
Component: Documentation
Component: FlightRPC
Component: GLib
Component: C#
Component: Go
Component: JavaScript
Component: MATLAB
Component: Ruby
Component: Swift
Component: Gandiva
Component: Parquet
awaiting review
labels
| (ArrowType.TimestampWithPrecision) vector.getField().getType(); | ||
| switch (type.getPrecision()) { | ||
| case 0: | ||
| this.unitsPerSecond = 1; |
There was a problem hiding this comment.
Why this have to be a member of the class? I feel these should be part of some kind of static lookup list.
| @@ -306,13 +306,17 @@ public void write(${name}Holder holder) { | |||
| ArrowType arrowType = new ArrowType.FixedSizeBinary(holder.byteWidth); | |||
| get${name}Writer(arrowType).setPosition(idx()); | |||
| get${name}Writer(arrowType).write(holder); | |||
| <#elseif minor.class == "TimestampWithPrecision"> | |||
| ArrowType arrowType = new ArrowType.TimeStampWithPrecision(holder.precision, null); | |||
There was a problem hiding this comment.
Since this also has timezone we might need a different name?
|
@xxlaykxx, do we think this change is something that the upstream community would accept? I'm not sure we want long term divergence from the upstream code base. I'm not a maintainer of this repo, though. |
|
Not sure about about community, we can try in any way. And if we want to support timestamps with arbitrary precision - we need this changes |
| @@ -129,7 +129,6 @@ jobs: | |||
| for python_package in $(brew list | grep python@); do | |||
There was a problem hiding this comment.
Let's try and make this change in dremio repo only.
We want to just add the Holder + Vector in Dremio and wire it up through the stack. Let's get just literals working as proof of concept.
There was a problem hiding this comment.
We also don't want to manage this fork in the arrow repo.
There was a problem hiding this comment.
Perhaps it will be similar to what we did for wiring VARIANT through the stack: https://github.com/dremio/dremio/pull/7291/files#
No description provided.