Snowflake Connectivity #9435

jdunkerley · 2024-03-14T18:45:06Z

Pull Request Description

Initial connection to Snowflake via an account, username and password.
Move make_column_fetcher to the Type_Mapping types.
Move the JDBC common stuff to Entity_Naming_Properties type.
Add database selector and read_single_column to Connection.
Tweaks to Postgres_Dialect to allow it to stand in for Snowflake_Dialect (temporary).
Added Standard.Snowflake library.
Snowflake_Connection has all the methods working.
Ability to read all primitives working (Timestamp with TimeZone untested at present).
Note: Snowflake_Spec not currently used as needs dialect fixes first.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

Add warehouses.

Update schema dropdowns.

…well.

Move some responsibilities to Type_Mapping.

distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Connection.enso

…nnection.enso Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Type_Mapping.enso

radeusgd · 2024-03-19T15:46:56Z

distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso

+   Based off Postgres for now.
+snowflake : Postgres_Dialect
+snowflake =
+    Postgres_Dialect.Value make_internal_generator_dialect "Snowflake" "Snowflake_Dialect" Snowflake_Type_Mapping


Ok for the PoC although in later iterations I'd consider adding some layer of abstraction - maybe we could define Postgres_Like_Dialect and have both Snowflake_Dialect and Postgres_Dialect "inherit" from it?

I think for next pass will just be a copy spun off from the Postgres_Dialect.
Eventually I'd like to have an Ansi_SQL_Like_Dialect but I dont have enough of a feel how best to do it yet.

Yes, it feels like we are lacking some good primitive for abstraction in Enso that would allow us to define the default behaviours on a variety of functions, but then override some of them for some of the dialects, while ideally still being able to delegate to the default from within an override under some conditions.

Essentially, regular Java-like class inheritance seems like a simplest abstraction, but we don't have that.

@JaroslavTulach maybe you have some ideas how we could achieve that from the perspective of designing a good internal API? We'd want to avoid having to write a ton of boilerplate code just to inherit the default behaviour.

Currently my 'best approximation' is the File_Write_Strategy which gives us defaults and then allows to override them by specifying custom functions. But with only ~5 methods it already is ugly and not too readable because all methods must be defined on a single line (as fields of the Value constructor) and it already introduces boilerplate. I don't feel like this approach would scale well to the complexity that Dialects have. But I don't yet know of anything that would work better either.

radeusgd · 2024-03-19T15:49:18Z

distribution/lib/Standard/Snowflake/0.0.0-dev/THIRD-PARTY/NOTICE

+Enso
+Copyright 2020 - 2024 New Byte Order sp. z o. o.
+
+'snowflake-jdbc', licensed under the The Apache Software License, Version 2.0, is distributed with the Snowflake.


Not related to this PR directly but I'm thinking if we should add a 'display name' for our components in the license review tool, as it would look much better if it were:

Suggested change

'snowflake-jdbc', licensed under the The Apache Software License, Version 2.0, is distributed with the Snowflake.

'snowflake-jdbc', licensed under the The Apache Software License, Version 2.0, is distributed with the Standard.Snowflake library.

distribution/lib/Standard/Database/0.0.0-dev/src/Internal/SQLite/SQLite_Type_Mapping.enso

distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Postgres/Postgres_Dialect.enso

test/Table_Tests/src/Database/Postgres_Spec.enso

distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Details.enso

test/Table_Tests/src/Database/Snowflake_Spec.enso

build.sbt

radeusgd

Looks like a great prototype for quickly getting Snowflake integrated.

I have a few issues (noted above) that I think need addressing, but I think it's fine to do them as a separate PR to be able to already benefit from the integration. The most important ones IMO are:

figuring out the type mapping to behave nicely with columns that report a small numeric type but still contain larger values,
refactoring the Postgres_Dialect to avoid snowflake specific if's inside of it, if possible. I'm not yet sure how to do this in the best way given that we don't have proper inheritance...

JaroslavTulach

I'd suggest using private a bit more.

JaroslavTulach · 2024-03-20T08:11:15Z

distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Dialect.enso

@@ -0,0 +1,12 @@
+from Standard.Base import all


Can this module be marked private?

yes will make the dialect private when I restructure it in next PR.

JaroslavTulach · 2024-03-20T08:11:30Z

distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Type_Mapping.enso

@@ -0,0 +1,182 @@
+private


JaroslavTulach · 2024-03-20T08:12:39Z

distribution/lib/Standard/Snowflake/0.0.0-dev/src/Snowflake_Details.enso

+       - warehouse: The name of the warehouse to use.
+    Snowflake account:Text credentials:Credentials database:Text="SNOWFLAKE" schema:Text="PUBLIC" warehouse:Text=""
+
+    ## PRIVATE


I'd suggest to move these ## PRIVATE methods to another private module and make them extension methods.

The methods on the details class make the Database.connect method work so need to be attached to the type.

JaroslavTulach · 2024-03-20T08:13:58Z

std-bits/database/src/main/java/org/enso/database/JDBCUtils.java

@@ -12,6 +12,17 @@
 import org.enso.polyglot.common_utils.Core_Date_Utils;

 public class JDBCUtils {


Missing private constructor. The default constructor is public hence people could create instances of this utility methods only class, which makes little sense.

JaroslavTulach · 2024-03-20T08:16:17Z

distribution/lib/Standard/Snowflake/0.0.0-dev/src/Internal/Snowflake_Connection.enso

@@ -0,0 +1,298 @@
+from Standard.Base import all


I guess only Snowflake_Details need to be visible from other modules. If so, I suggest to make this module private.

JaroslavTulach · 2024-03-20T08:19:48Z

test/Table_Tests/src/Database/Snowflake_Spec.enso

+
+    connection self = self.data.at 0
+    tinfo self = self.data.at 1
+    t self = self.data.at 2


I do recognize the group laziness pattern. Nice.

The #9435 broke the notarization, as it adds new binaries to the package that need to be included in our code signing workarounds.

jdunkerley added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Mar 15, 2024

jdunkerley and others added 12 commits March 18, 2024 10:58

Initial connection to Snowflake via an account, username and password.

d4f19cb

Fix databases and schemas in Snowflake.

7eae8c8

Add warehouses.

Add warehouse.

c20618c

Update schema dropdowns.

Add ability to set warehouse and pass at connect.

e6d1075

Fix for NPE in license review

fb8b8ee

scalafmt

f6b1f11

Separate Snowflake from Database.

e1c3602

Scala fmt.

7f4b38b

Legal Review

957a308

Avoid using ARROW for snowflake.

08c8148

Tidy up Entity_Naming_Properties.

ac999c8

Fix for separating Entity_Namimg_Properties.

a0c2b77

jdunkerley force-pushed the wip/jd/table-fixes branch from f121b79 to a0c2b77 Compare March 18, 2024 13:17

jdunkerley added 5 commits March 18, 2024 14:59

Allow some tweaking of Postgres dialect to allow snowflake to use as …

df5a348

…well.

Working on reading Date, Time and Date Times.

4752e1c

Changelog.

4ff2bc5

Java format.

309d2ee

Make Snowflake Time and TimeStamp stuff work.

e4e0160

Move some responsibilities to Type_Mapping.

jdunkerley marked this pull request as ready for review March 19, 2024 13:41

jdunkerley requested review from radeusgd, GregoryTravis, AdRiley, 4e6, JaroslavTulach, hubertp and Akirathan as code owners March 19, 2024 13:41

Make Snowflake Time and TimeStamp stuff work.

637b456

Move some responsibilities to Type_Mapping.