-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-315: finalize timestamp #156
Conversation
@@ -64,8 +64,11 @@ table Date { | |||
table Time { | |||
} | |||
|
|||
enum TimeUnit: short { MILLISECOND, MICROSECOND } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add nanosecond? This is quite common in the financial world for example (part of the reason that pandas uses nanoseconds)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, Unix timestamps are seconds since the epoch. Not sure how commonly occurring these are in practice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for adding both, SECOND and NANOSECOND
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added millis and micros because those are the ones that Parquet supports.
For now nanos and second metadata would be lost in saving to Parquet and stored as int?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine to drop the metadata in those cases when writing to Parquet. You would need to write key-value metadata to be able to recover the exact metadata (timestamp in nanos)
table Timestamp { | ||
timezone: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the thought that a time zone (if any) would go in the custom metadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timezoned data should be converted to UTC before putting in arrow.
That's simpler.
We can add timezone later on if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine with me. My experience has been that having both time zone naive and non-naive timestamps is more trouble than it's worth
+1, thank you |
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer Change-Id: Idaef448d5a5ac4b29794f141d2b5c87a1e4f40d5
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer Change-Id: Idaef448d5a5ac4b29794f141d2b5c87a1e4f40d5
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer Change-Id: Idaef448d5a5ac4b29794f141d2b5c87a1e4f40d5
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer Change-Id: Idaef448d5a5ac4b29794f141d2b5c87a1e4f40d5
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer Change-Id: Iabe80b1cbe3fd8f1de6239187058b6402b160975
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer Change-Id: Iabe80b1cbe3fd8f1de6239187058b6402b160975
I wrote a sample file and the metadata seems to be correct. @xhochy I fixed some missing metadata like `dictionary_page_offset`. You might want to check if this fixes the Drill problem. Author: Deepak Majeti <deepak.majeti@hpe.com> Closes #156 from majetideepak/PARQUET-711 and squashes the following commits: 25f5a7e [Deepak Majeti] fix schema and descr. Resolves PARQUET-705 and PARQUET-707 8b4784d [Deepak Majeti] Review comments to add methods back fdbc761 [Deepak Majeti] fix clang error and comments c6cb071 [Deepak Majeti] convert DCHECKS to Exceptions in metadata ada3ac2 [Deepak Majeti] clang format d9c9131 [Deepak Majeti] Use metadata builders in parquet writer Change-Id: Iabe80b1cbe3fd8f1de6239187058b6402b160975
…t-get-sql-info [Flight SQL CPP] Add support for command `GetSqlInfo` on client side
No description provided.