-
Notifications
You must be signed in to change notification settings - Fork 336
Tableau Dual JVM tweaks #14631
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
base: develop
Are you sure you want to change the base?
Tableau Dual JVM tweaks #14631
Conversation
std-bits/tableau/src/main/java/org/enso/tableau/HyperFormat.java
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Table/0.0.0-dev/src/In_Memory_Table.enso
Outdated
Show resolved
Hide resolved
std-bits/table/src/main/java/org/enso/table/data/column/storage/type/StorageType.java
Outdated
Show resolved
Hide resolved
dd725c5 to
75169be
Compare
JaroslavTulach
left a comment
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'd improve encapsulation with private especially in methods that return Java classes not transferable over dual JVM boundary
distribution/lib/Standard/Table/0.0.0-dev/docs/api/Internal/In_Memory_Helpers.md
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Table/0.0.0-dev/docs/api/Internal/In_Memory_Helpers.md
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Table/0.0.0-dev/docs/api/Internal/In_Memory_Helpers.md
Show resolved
Hide resolved
std-bits/database/src/main/java/org/enso/database/fetchers/ColumnFetcherFactory.java
Show resolved
Hide resolved
std-bits/table/src/main/java/org/enso/table/data/column/builder/Builder.java
Outdated
Show resolved
Hide resolved
std-bits/table/src/main/java/org/enso/table/data/column/operation/cast/CastOperation.java
Outdated
Show resolved
Hide resolved
std-bits/tableau/src/main/java/org/enso/tableau/HyperFormat.java
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Java_Problems.enso
Outdated
Show resolved
Hide resolved
fedd26b to
310fc71
Compare
|
| problems = if warning_unmatched_columns.length > 0 then [Unmatched_Columns.Error warning_unmatched_columns.to_vector] else [] | ||
| on_problems.attach_problems_before problems <| | ||
| File.new resolved_file | ||
| if (on_existing_file == Existing_File_Behavior.Error) && file.exists then Error.throw (File_Error.Already_Exists file) |
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.
- wow, this is relaying on a fact that statements that yield
Errorterminate block execution! - by having
if_thenonly (and notif_then_else) you eliminate one indentation level - but it feels less FP-like... which is probably not a problem at all
Add a temporary makeLocalStorage method until JavaProblems is rewritten.
… accessing data from other libraries.
DONT LIKE THIS - its leaking too much.
…geType is always local.
Add test.
d42041e to
f971bf3
Compare
Pull Request Description
First PR following up on #14607
Small fix for
File.newwidget.Renamed
In_Memory_Test_HelperstoIn_Memory_Helpers.Make use of
get_java_tablemore explicitly from the helper class.StorageType "localisation" should be internal
Storage.to_value_type.makeLocalTypemethod to help with this.Currently in
Java_ProblemsandIn_Memory_Column_Implementation.In_Memory_Table.java_tableshould always be within the primary runtime.HyperFormat.writeTabletakes an array of names and an array of storages (ColumnStorageinterface).Altered external use of Java Column and Table to only use the ColumnStorage.
Removed redundant helper methods.
Pass names and storages and avoid passing the Table object.
Next PR: Overhaul exception handling.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.