Port DH-11692: Add native support for java.time types#3385
Merged
nbauernfeind merged 14 commits intodeephaven:mainfrom Mar 3, 2023
Merged
Port DH-11692: Add native support for java.time types#3385nbauernfeind merged 14 commits intodeephaven:mainfrom
nbauernfeind merged 14 commits intodeephaven:mainfrom
Conversation
nbauernfeind
added
query engine
DocumentationNeeded
port
ReleaseNotesNeeded
cpwright
reviewed
Feb 2, 2023
nbauernfeind
force-pushed
the
dh_11692
branch
from
3de7432 to
b0a0f12
Compare
cpwright
previously approved these changes
Feb 3, 2023
rcaudy
reviewed
Feb 15, 2023
nbauernfeind
force-pushed
the
dh_11692
branch
from
515286e to
a3efc5d
Compare
rcaudy
reviewed
Feb 22, 2023
Contributor
Author
|
I can't seem to reply to your final comment. I added ZonedDateTime/Instant to the list. I removed Date. For now LocalDate and LocalTime are stubbed with FixedSizeBinary formats, but these are not implemented between server/client neither for worker-to-worker nor for the jsapi. At this moment, barrage strips the ZoneId from ZonedDateTime columns and sends as UTC. The ser/deser code acts on Class<?> and it would be much nicer to use ColumnDataType instead of passing extra args. |
nbauernfeind
added a commit
to deephaven/web-client-ui
that referenced
this pull request
Feb 28, 2023
deephaven/deephaven-core#3385 is a port of DH-11692 which adds `Instant` and `ZonedDateTime` support to the engine. I've similarly encoded these types over Barrage as a `long` (as a nanosecond since epoch). The encoding of Zones will actually need to be implemented at a future date as the best implementation requires (encourages?) a much larger refactoring of ColumnSource types. See deephaven/deephaven-core#3455 for more information. I was able to get the web-client-ui to display these types with this small patch.
rcaudy
reviewed
Mar 2, 2023
…tMemoryColumnSource
…tSparseMemoryColumnSource
rcaudy
approved these changes
Mar 3, 2023
Contributor
|
Labels indicate documentation is required. Issues for documentation have been opened: How-to: https://github.com/deephaven/deephaven.io/issues/2256 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I did a bit more replication than was in the DHE implementation; but hopefully for the better.
DH-11692 via commits:
Follow up work:
Instant/LocalDate/LocalTime/ZonedDateTimesupport for Parquet, KafkaTools and JsonNodeUtil #3367ArraySource#movesupport moving of full blocks #3359ColumnDataTypeto use as return type toColumnSource#getType#3455Nightlies: https://github.com/nbauernfeind/deephaven-core/actions/runs/4296250838