Add storage support for Date, Time and DateTime to InMemory table #3673
Conversation
This file contains 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
radeusgd
force-pushed
the
wip/radeusgd/in-memory-table-datetime-storage-183080911
branch
from
d1fcc89
to
9b542d6
Compare
radeusgd
force-pushed
the
wip/radeusgd/in-memory-table-datetime-storage-183080911
branch
from
ba7fba2
to
59c8652
Compare
radeusgd
force-pushed
the
wip/radeusgd/in-memory-table-datetime-storage-183080911
branch
from
59c8652
to
a37f2f5
Compare
jdunkerley
approved these changes
Aug 31, 2022
distribution/lib/Standard/Table/0.0.0-dev/src/Data/Data_Formatter.enso
Outdated
Show resolved
Hide resolved
| import org.enso.table.data.column.storage.StringStorage; | ||
|
|
||
| /** A column builder appending all the values passed to it in an unchanged form. */ | ||
| public class StringStorageBuilder extends StorageBuilder { | ||
|
|
||
| private Object[] data; | ||
| private String[] data; |
There was a problem hiding this comment.
Is us doing this better than just using an ArrayList<T> feels like we are doing this for little gain once especially we have the new polyglot arrays
There was a problem hiding this comment.
Hmm, well, to some extent. I'd argue that our implementation is still more specialized. It avoids copying (whereas ArrayList<T>::toArray would copy IIRC. And it has the retype logic which we would still need to add.
So I'd be for keeping it as-is, although you are right that there is quite a lot of similarity.
std-bits/table/src/main/java/org/enso/table/data/column/builder/object/InferredBuilder.java
Outdated
Show resolved
Hide resolved
std-bits/table/src/main/java/org/enso/table/data/column/builder/object/TimeOfDayBuilder.java
Outdated
Show resolved
Hide resolved
WIP: somehow use_first_row_as_names broke
radeusgd
force-pushed
the
wip/radeusgd/in-memory-table-datetime-storage-183080911
branch
from
a37f2f5
to
6923e4c
Compare
radeusgd
added
CI: Ready to merge
mergify
bot
deleted the
wip/radeusgd/in-memory-table-datetime-storage-183080911
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pull Request Description
Implements https://www.pivotaltracker.com/story/show/183080911
Important Notes
Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide buildand./run ide watch.