-
Notifications
You must be signed in to change notification settings - Fork 334
Run Standard.Snowflake in dual JVM mode
#14474
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?
Conversation
|
There is currently 40 failures: need to be addressed. |
|
|
14 failures remaining: |
distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Statement_Setter.enso
Outdated
Show resolved
Hide resolved
Standard.Snowflake in dual JVM mode
Standard.Snowflake in dual JVM modeStandard.Snowflake in dual JVM mode
|
With e59ef9c we are down to 10 failures: |
… Columns is prepared. Make them lazy!
|
With 3907ebf we seems to be down at a single failure when executed locally: but then the extra run reports: ❌ should see Database operations performed on a table through the standard workflow ✅ materialize should respect the overridden type ❌ [Snowflake] Key-pair authentication ✅ using local private key |
| suite_builder.group prefix+"Table.Offset with default fill strategy" group_builder-> | ||
| colA = ["A", [1, 2, 3]] | ||
| t1 = build_sorted_table [colA] | ||
| sorted = Sorted_Table.Ordered (build_sorted_table [colA]) |
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.
- running single snowflake test takes ages
sbt:enso> runEngineDistribution --run test/Snowflake_Tests/ Decimal.and.Float.types
- because it initializes more than two thousands of
Columninstances! - a329d86 is the current recommended solution
- I changed the code in
Offset_Spec, mostly introduced by Add Table.Offset #12071, @AdRiley - however there are many other test files initializing columns too eagerly
- I changed the code in
- do we want to change all of them to use the suspended atom fields as introduced by Suspended atom fields are evaluated only once #6151?
| BuilderUtil.LOG.trace( | ||
| "makeLocal unsuccessful for {}:{} size {}", t.typeChar(), t.size(), storage.getSize()); | ||
| switch (localType) { | ||
| case BigIntegerType _ -> { |
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.
sbt:enso> runEngineDistribution
--vm.D=polyglot.enso.classLoading=Standard.Snowflake:guest,hosted
--env ENSO_SNOWFLAKE_USER=aaa
--env ENSO_SNOWFLAKE_PASSWORD=bbb
--env ENSO_SNOWFLAKE_ACCOUNT=ccc
--env ENSO_SNOWFLAKE_DATABASE=CI_TEST_DB
--run test/Snowflake_Tests should.be.able.to.round-trip.a.BigInteger.column
[FAILED] [Snowflake] Edge Cases: [0/1, 2127ms]
- [FAILED] should be able to round-trip a BigInteger column [2127ms]
Reason: (1.1805916207174113E21): Expected a value of type Standard.Base.Data.Numbers.Integer
but got a value [1.1805916207174113E21] of type Standard.Base.Data.Numbers.Float instead
(at enso/enso/test/Snowflake_Tests/src/Snowflake_Spec.enso:375:17-37).
- the problem is that with Proxy
Storagethere is Truffle value conversion - when obtaining values from the storage - e.g. when calling from Enso to Java
- and
BigIntegermay be returned asdoubleorfloat - when it can be exactly represented by
doubleorfloat
- and
- by eagerly copying the content of the storage to local JVM we convert everything back to
BigInteger - conversions to big integer had to be improved
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.
- it would be faster to use off-heap memory transfer
- but I don't know what's appropriate Arrow format for
java.math.BigInteger!? - there is Int+bit width, but the width can be 64 bits at max...
| try { | ||
| return switch (o) { | ||
| case BigInteger big -> big; | ||
| case Double d -> new BigDecimal(d).toBigIntegerExact(); |
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.
- this models the HostObject conversion for double
- and also conversion for float
| var isProxy = Proxy.isProxyClass(storage.getClass()); | ||
| this.storage = isProxy ? Builder.makeLocal(storage) : storage; | ||
| var type = this.storage.getType(); | ||
| LOG.debug( |
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.
- Run any tests with
org.enso.table.Logger.level=traceto seeColumninstances being created - for example running:
sbt:enso> runEngineDistribution
--vm.D=org.enso.table.Logger.level=trace
--env ENSO_SNOWFLAKE_USER=aaa
--env ENSO_SNOWFLAKE_PASSWORD=bbb
--env ENSO_SNOWFLAKE_ACCOUNT=ccc
--env ENSO_SNOWFLAKE_DATABASE=CI_TEST_DB
--run test/Snowflake_Tests nicnicnic
8 tests succeeded.
0 tests failed.
0 tests skipped.
0 groups skipped.- initializes all
Snowflake_Tests, but runs none - if any
Columnis created, then it is premature - a25e91c delays the initialization a lot, but not fully
- related to Make sure suspended blocks are evaluated only once #14530 @Akirathan, @jdunkerley
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.
- a25e91c delays the initialization a lot, but not fully
- follow up is at Make
Standard.Testpendingfield lazy #14536
Pull Request Description
ensonative image executable.Important Notes
Execute in mock dual JVM mode via:
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Java,