Add Snowflake connector #17909

dprophet · 2023-06-14T20:33:28Z

Description

New Snowflake connector

Equal acknowledgements go to

Erik Anderson, Bloomberg
Yu Teng, ForePaaS

This Snowflake connector is a collaboration between both individuals at separate companies. Developing and maintaining such a connector has taken about 2 years of efforts. The community has came together to help solve challenge that is greater than 1 person, or 1 company. A lot of time, energy has gone into creating, maintaining, and open sourcing this connector.

This work references pull request #10387

ebyhr

Just skimmed.

Please rebase on master and squash commits into one.

ebyhr · 2023-06-14T22:19:05Z

plugin/trino-snowflake/pom.xml

+    <parent>
+        <groupId>io.trino</groupId>
+        <artifactId>trino-root</artifactId>
+        <version>413-SNAPSHOT</version>


Please update this version.

Updated to 422-SNAPSHOT

ebyhr · 2023-06-14T22:48:47Z

plugin/trino-snowflake/pom.xml

@@ -0,0 +1,180 @@
+<?xml version="1.0"?>


Please update ci.yml.

ebyhr · 2023-06-14T22:49:32Z

plugin/trino-snowflake/pom.xml

+        </dependency>
+    </dependencies>
+
+    <build>


Please add cloud-tests profile. You can refer to other SaaS connector. e.g. BigQuery

ebyhr · 2023-06-14T22:50:27Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+    private final Type jsonType;
+    private final AggregateFunctionRewriter aggregateFunctionRewriter;


nit: Move to under static final fields.

ebyhr · 2023-06-14T22:50:45Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+    private static final DateTimeFormatter snowballDateTimeFormatter = DateTimeFormatter.ofPattern("y-MM-dd'T'HH:mm:ss.SSSSSSSSSXXX");
+    private static final DateTimeFormatter snowballDateFormatter = DateTimeFormatter.ofPattern("uuuu-MM-dd");
+    private static final DateTimeFormatter snowballTimestampFormatter = DateTimeFormatter.ofPattern("y-MM-dd'T'HH:mm:ss.SSSSSSSSS");
+    private static final DateTimeFormatter snowballTimeFormatter = DateTimeFormatter.ofPattern("HH:mm:ss.SSSSSSSSS");


Rename prefix to snowflake and uppercase these constants.

ebyhr · 2023-06-14T23:09:54Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java

+import static io.trino.spi.type.VarcharType.createVarcharType;
+import static java.time.ZoneOffset.UTC;
+
+public class TestSnowflakeTypeMapping


Add test case for timestamp types.

Already exist at line 308:
@Test public void testTimestamp() { testTimestamp(UTC); testTimestamp(jvmZone); testTimestamp(vilnius); testTimestamp(kathmandu); testTimestamp(TestingSession.DEFAULT_TIME_ZONE_KEY.getZoneId()); }

It's timestamp without time zone. timestamp with time zone test is still missing.

ebyhr · 2023-06-14T23:10:36Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+    private static LongWriteFunction timestampWriter()
+    {
+        return (statement, index, value) -> statement.setString(index, StandardColumnMappings.fromTrinoTimestamp(value).toString());
+    }
+
+    private static Optional<ColumnMapping> getUnsignedMapping(JdbcTypeHandle typeHandle)


Remove unused methods.

ebyhr · 2023-06-14T23:12:09Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    public void testCharVarcharComparison()
+    {
+        // Override and skip it because snowflake not support this feature
+        assertThatThrownBy(super::testCharVarcharComparison).isInstanceOf(AssertionError.class);


Please verify the message instead of the class instance. Same for other places.

Updated for all tests

ebyhr · 2023-06-14T23:13:11Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    @Override
+    public void testCreateTable()
+    {
+        // Override and skip it because snowflake not support this feature


Snowflake supports creating a table. This comment should be updated. Same for other places.

Override it. In fact there is only one thing that null doesn't equal to "". So I override it

ebyhr · 2023-06-14T23:15:01Z

docs/src/main/sphinx/connector/snowflake.rst

+``char(n)``                         ``varchar(n)``
+``binary(n)``                       ``varbinary``
+``varbinary``                       ``varbinary``
+``date``                            ``date``


timestamp type is missing.

ebyhr · 2023-06-14T23:20:03Z

docs/src/main/sphinx/connector/snowflake.rst

+-------------
+
+To configure the Snowflake connector, create a catalog properties file
+in ``etc/catalog`` named, for example, ``snowflake.properties``, to


We use example.properties as the template. See other connector docs.

Updated

Did you forget to push the changes?

Because this PR is created from the repository of Erik, I can't maintain and update it directly. I have to send him a new PR first, and after he accepts it, you will see the changes. I'll let you know when it has done.

I'll let you know when it has done.

Please avoid leaving "Updated" comments until this PR is updated. The comment is confusing and not actionable.

ebyhr · 2023-06-15T01:24:31Z

plugin/trino-snowflake/pom.xml

+        <dependency>
+            <groupId>net.snowflake</groupId>
+            <artifactId>snowflake-jdbc</artifactId>
+            <version>3.13.29</version>


The latest version is 3.13.32.

ebyhr · 2023-06-15T01:31:01Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    public void testCreateTable()
+    {
+        // Override and skip it because snowflake not support this feature
+        assertThatThrownBy(super::testCreateTable).isInstanceOf(AssertionError.class);


We shouldn't ship untested features to users. I would recommend making this connector read-only by overriding some methods in SnowflakeClient. e.g. createTable

Maybe we can support createTable.

bitsondatadev · 2023-07-10T15:56:41Z

@dprophet - @yuuteng any updates here?

yuuteng · 2023-07-10T16:55:10Z

@dprophet - @yuuteng any updates here?

I will do PR to @dprophet about these issues mentioned.

yuuteng

Checked all reviews and I will update a PR in the next comment

yuuteng · 2023-07-11T08:12:36Z

docs/src/main/sphinx/connector/snowflake.rst

+-------------
+
+To configure the Snowflake connector, create a catalog properties file
+in ``etc/catalog`` named, for example, ``snowflake.properties``, to


Because this PR is created from the repository of Erik, I can't maintain and update it directly. I have to send him a new PR first, and after he accepts it, you will see the changes. I'll let you know when it has done.

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeConfig.java

yuuteng · 2023-07-13T12:40:04Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeConfig.java

+        return Optional.ofNullable(timestampNoTimezoneAsUTC);
+    }
+
+    @Config("snowflake.timestampNoTimezoneAsUtc")


yuuteng · 2023-07-13T12:41:13Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+    private final Type jsonType;
+    private final AggregateFunctionRewriter aggregateFunctionRewriter;


yuuteng · 2023-07-13T12:45:49Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+    private static final DateTimeFormatter snowballDateTimeFormatter = DateTimeFormatter.ofPattern("y-MM-dd'T'HH:mm:ss.SSSSSSSSSXXX");
+    private static final DateTimeFormatter snowballDateFormatter = DateTimeFormatter.ofPattern("uuuu-MM-dd");
+    private static final DateTimeFormatter snowballTimestampFormatter = DateTimeFormatter.ofPattern("y-MM-dd'T'HH:mm:ss.SSSSSSSSS");
+    private static final DateTimeFormatter snowballTimeFormatter = DateTimeFormatter.ofPattern("HH:mm:ss.SSSSSSSSS");


yuuteng · 2023-07-13T13:11:21Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+    private static LongWriteFunction timestampWriter()
+    {
+        return (statement, index, value) -> statement.setString(index, StandardColumnMappings.fromTrinoTimestamp(value).toString());
+    }
+
+    private static Optional<ColumnMapping> getUnsignedMapping(JdbcTypeHandle typeHandle)


yuuteng · 2023-07-13T13:19:37Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+//            .put(Types.TIME, columnMappingPushdown(timeColumnMapping()))
+//            .buildOrThrow();
+
+    private static final Map<String, ColumnMappingFunction> SHOWFLAKE_COLUMN_MAPPINGS = ImmutableMap.<String, ColumnMappingFunction>builder()


Don't understand what do you mean "Inline" ? Can you explain about what do I need to do or do you have an example about it please?

yuuteng · 2023-07-13T14:19:17Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    public void testCharVarcharComparison()
+    {
+        // Override and skip it because snowflake not support this feature
+        assertThatThrownBy(super::testCharVarcharComparison).isInstanceOf(AssertionError.class);


Updated for all tests

yuuteng · 2023-07-13T14:20:24Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    @Override
+    public void testCreateTable()
+    {
+        // Override and skip it because snowflake not support this feature


Override it. In fact there is only one thing that null doesn't equal to "". So I override it

yuuteng · 2023-07-13T14:21:25Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    public void testCreateTable()
+    {
+        // Override and skip it because snowflake not support this feature
+        assertThatThrownBy(super::testCreateTable).isInstanceOf(AssertionError.class);


Maybe we can support createTable.

yuuteng · 2023-07-13T16:43:28Z

@ebyhr Here is my new pull request : bloomberg#7
It needs to be merged by @dprophet. I let you take a look and merge it.

chenjian2664 · 2023-08-07T08:02:07Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java

+    public Object[][] snowflakeIntegerTypeProvider()
+    {
+        // INT , INTEGER , BIGINT , SMALLINT , TINYINT , BYTEINT, DECIMAL , NUMERIC are aliases for NUMBER(38, 0) in snowflake
+        // https://docs.snowflake.com/en/sql-reference/data-types-numeric.html#int-integer-bigint-smallint-tinyint-byteint


Add Unsupported test cases, e.g: testUnsupportedTinyint with value min-1 and max+

chenjian2664 · 2023-08-07T08:02:21Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java

+                .addRoundTrip("decimal(24, 4)", "CAST('12345678901234567890.31' AS decimal(24, 4))", createDecimalType(24, 4), "CAST('12345678901234567890.31' AS decimal(24, 4))")
+                .addRoundTrip("decimal(30, 5)", "CAST('3141592653589793238462643.38327' AS decimal(30, 5))", createDecimalType(30, 5), "CAST('3141592653589793238462643.38327' AS decimal(30, 5))")
+                .addRoundTrip("decimal(30, 5)", "CAST('-3141592653589793238462643.38327' AS decimal(30, 5))", createDecimalType(30, 5), "CAST('-3141592653589793238462643.38327' AS decimal(30, 5))")
+//                .addRoundTrip("decimal(38, 0)", "CAST('27182818284590452353602874713526624977' AS decimal(38, 0))", createDecimalType(38, 0), "CAST('27182818284590452353602874713526624977' AS decimal(38, 0))")


chenjian2664 · 2023-08-07T08:06:35Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    @Override
+    public void testReadMetadataWithRelationsConcurrentModifications()
+    {
+        throw new SkipException("Test fails with a timeout sometimes and is flaky");


Is it possible open the test and annotated with @Flaky instead of skipping the test?

chenjian2664 · 2023-08-07T08:09:31Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+    public void testDropAmbiguousRowFieldCaseSensitivity()
+    {
+        // Override and skip it because snowflake not support this feature
+        assertThatThrownBy(super::testDropAmbiguousRowFieldCaseSensitivity);


It would be better to add more checks like using hasMessage call

chenjian2664 · 2023-08-07T08:14:58Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeTypeMapping.java

+    public void testBinary()
+    {
+        SqlDataTypeTest.create()
+                .addRoundTrip("binary(18)", "NULL", VARBINARY, "CAST(NULL AS varbinary)")


We can extract the common test cases like the TestIgniteTypeMapping.binaryTest

If it passes, can we leave this place for the moment ? I'll optimize it later.

chenjian2664 · 2023-08-07T08:18:39Z

plugin/trino-snowflake/src/test/java/io/trino/plugin/snowflake/TestSnowflakeConnectorTest.java

+                        "   shippriority bigint,\n" +
+                        "   comment varchar(79)\n" +
+                        ")\n" +
+                        "COMMENT ''");


Why here has COMMENT field even we declare not support SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT and SUPPORTS_COMMENT_ON_TABLE

chenjian2664 · 2023-08-07T08:25:07Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+        return true;
+    }
+
+    private ColumnMapping jsonColumnMapping()


Why add this?

Please remove this method.

afranzi · 2023-09-22T17:31:49Z

Hello team, wondering how active is this PR and how much effort is required to make it happen 🚀 ?

ebyhr · 2023-11-01T04:26:13Z

Files changed 133

Please revert unrelated changes. Also, please squash commits into one.

luisfalconeri · 2023-11-23T15:59:11Z

Hello there! Firstly, nice work :-) Wondering if there are any updates on the status here? Thanks a lot

hongbo-miao · 2023-11-29T08:37:58Z

Hi @ebyhr, considering the substantial size of this pull request, squashing it early might pose challenges for ongoing work, such as addressing conflicts. Once the pull request ready, it would be great to have the team's approval and proceed with a squash merge. Looking forward to a merge soon! ☺️

yuuteng · 2023-11-29T13:26:19Z

After updating this PR with the latest trino version, there are some new errors in the previous tests. I am trying to fix them and also I'm trying to reach @dprophet to have a period of time free to review and merge it. Because as we know that with the upgrade of version there are new issues arising. We have to merge it asap after I fix again. 😄

bitsondatadev · 2023-11-29T16:48:13Z

@colebow @mosabua I am a bit pressed for time, this week but I would be available in the following weeks to help land this. Would one of you either mind starting to reach out to everyone involved to create a plan on landing this?

I think at this point we need to coordinate what is needed on the PR with @ebyhr, when Yuya and @dprophet will have availability to quickly review, and synchronize the update and review process around the same times to avoid long pauses that then require large merges and breaks previous iterations. That's certainly not fun for @yuuteng to keep up. This is a brand new connector that has quite a few extra concerns around testing and I think a coordinated effort will help this last mile from stagnating. Let me know if either of you are up for it, otherwise happy to raise my hand to coordinate this in the coming weeks.

mosabua · 2023-11-29T18:26:59Z

Agreed .. we really should try to get this over the finish line .. ideally before Trino Summit in two weeks ;-)

I don't know about availability for these efforts from @dprophet and @yuuteng and where the current hold up is - next step is probably to resolve the conflicts.

bitsondatadev · 2023-11-29T22:32:00Z

Right but I think @yuuteng doesn't want to keep doing this unless he knows this will the The Last Merge Conflict™.

So I think having DevRel do some upfront coordination to make sure Teng isn't wasting his time would be good at this point.

yuuteng · 2023-12-04T09:15:05Z

@bitsondatadev Thanks for understanding. I will take some time to fix it today and I think I may need some help from @ebyhr . As for the process, I think it will be better if I create a new PR directly from my repository and add @dprophet as reviewer inside so that I can save time to find a time with @dprophet first, and he spent also time to resolve the conflicts. I guess it will not be finished in one commit. In this way, it would be more efficient. I'm talking about it with him at the same time for his opinion.

ebyhr · 2024-02-26T22:36:17Z

Please fix CI failures.

errorprone

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.12.1:compile (default-compile) on project trino-snowflake: Compilation failure: Compilation failure: 
Error:  /home/runner/work/trino/trino/plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java:[367,16] [UnusedVariable] The local variable 'jdbcTypeName' is never read.
Error:      (see https://errorprone.info/bugpattern/UnusedVariable)
Error:    Did you mean to remove this line or 'typeHandle.getJdbcTypeName()'?
Error:  /home/runner/work/trino/trino/plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java:[369,13] [UnusedVariable] The local variable 'type' is never read.
Error:      (see https://errorprone.info/bugpattern/UnusedVariable)
Error:    Did you mean to remove this line or 'typeHandle.getJdbcType();'?

trino-snowflake

Error:  Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.2.5:test (default-test) on project trino-snowflake: No tests were executed!  (Set -DfailIfNoTests=false to ignore this error.) -> [Help 1]

trino-snowflake cloud-tests

Error:  Errors: 
Error:    TestSnowflakeConnectorTest>AbstractTestQueryFramework.init:113->createQueryRunner:29 » ExceptionInInitializer
Error:    TestSnowflakeTypeMapping>AbstractTestQueryFramework.init:113->createQueryRunner:78 » NoClassDefFound Could not initialize class io.trino.plugin.snowflake.TestingSnowflakeServer

and product tests

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

***** Update ci and delete no use lines (#20) Approved Update according to reviews 11/01/2024 Various style fixes and cleanup (#15) (#17) Co-authored-by: Martin Traverso <[email protected]> Various style fixes and cleanup (#15) Update the github CI (#12) * Add Snowflake JDBC Connector * Add snowflake in the ci Add Snowflake JDBC Connector (#11) Had to redo the connector because all the rebases caused havoc

lxynov · 2024-03-05T06:30:56Z

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java

+
+        // Mappings for JDBC column types to internal Trino types
+        final Map<Integer, ColumnMapping> standardColumnMappings = ImmutableMap.<Integer, ColumnMapping>builder()
+                .put(Types.BOOLEAN, StandardColumnMappings.booleanColumnMapping())


This needs to be updated.

For example, types INT, INTEGER, BIGINT, SMALLINT, TINYINT, BYTEINT are actually synonymous with NUMBER(38, 0) in Snowflake (ref). Their internal "data_type" is always a constant FIXED (ref). When they're read in via JDBC, their SQL type depends on Snowflake session property JDBC_TREAT_DECIMAL_AS_INT. By default its true, so the SQL type is always Types.BIGINT. This logic is in Snowflake JDBC code: https://github.com/snowflakedb/snowflake-jdbc/blob/d4ca9f0e99b142fa0c1a3dffe5cc7bde4fdfaf61/src/main/java/net/snowflake/client/jdbc/SnowflakeUtil.java#L177.

So:

We actually don't need mappings for INT, INTEGER, SMALLINT, TINYINT, etc. as they will never be used.

There will be an issue if a user creates an BIGINT column in Snowflake and uses it to store a value larger than Long.MAX_VALUE.

You can reproduce the example issue:

In Snowflake

CREATE TABLE testdb.test1.t_large_bigint (c_bigint BIGINT); INSERT INTO testdb.test1.t_large_bigint VALUES(99999999999999999999999999999999999998);

This query

SELECT * FROM testdb.test1.t_large_bigint

will succeed in Snowflake but fails in current connector implementation with an error

Query 20240305_052805_00005_6tebh failed: Cannot convert value in the driver from type:FIXED(null,null) to type:Long, value=99999999999999999999999999999999999998.

A safer way is to set JDBC_TREAT_DECIMAL_AS_INT to false and always read in decimal values.

^The above is an example. It applies to other data types, i.e., DOUBLE is used for all floating-point numbers, VARCHAR is used for all text types, etc. We will have to reference both Snowflake doc (here and here) and JDBC code for source-of-truths so that type conversion correctness is ensured: https://github.com/snowflakedb/snowflake-jdbc/blob/d4ca9f0e99b142fa0c1a3dffe5cc7bde4fdfaf61/src/main/java/net/snowflake/client/jdbc/SnowflakeUtil.java#L205-L321

cc @ebyhr @dprophet @yuuteng

I will try to fix it when I have time. If you can give me a PR directly, it will be helpful. 😄
@dprophet If you have time you can also take a look on it.

Thank you @yuuteng ! Actually I was working on co-authoring a new one but it took longer than I expected. I just commented here in case you folks are working on updates in parallel. I'll see how much I can get it done this week, btw please feel free to work on it on your side as well, I could help comment in that case.

As you can see that I am still struggling to work on the fixes about tests and ci problem. I will come back to see this when I clean them. 😄

@lxynov Lets not fix this right now. Connectors dont need to be perfect on day one. This PR has been going on for over a year. We can do supplimental PR's once this one is merged.

@dprophet Skipping this comment is fine. However, please file an issue to https://github.com/trinodb/trino/issues/new

Done. #20977

ebyhr · 2024-03-05T10:26:43Z

@yuuteng Please fix the following failures:

Error:  Failures: 
Error:    TestSnowflakeConnectorTest>BaseConnectorTest.testCommentTable:3914 
expected: null
 but was: ""
Error:    TestSnowflakeConnectorTest>BaseConnectorTest.testDataMappingSmokeTest:5579->BaseConnectorTest.testDataMapping:5611->AbstractTestQueryFramework.assertQuery:350 Execution of 'actual' query 20240305_101440_01101_pf2sh failed: SELECT row_id FROM test_data_mapping_smoke_timestampas9yk3wb3e WHERE rand() = 42 OR value IS NULL
Error:    TestSnowflakeConnectorTest>BaseConnectorTest.testSelectInformationSchemaColumns:2107->AbstractTestQueryFramework.assertQuery:350 Execution of 'actual' query 20240305_100625_00287_pf2sh failed: SELECT table_schema FROM information_schema.columns WHERE table_schema = 'tpch' GROUP BY table_schema
Error:  Errors: 
Error:    TestSnowflakeConnectorTest>BaseConnectorTest.testCommentTableSpecialCharacter:5522->BaseConnectorTest.testCommentTableSpecialCharacter:5538->AbstractTestQueryFramework.assertUpdate:405->AbstractTestQueryFramework.assertUpdate:410 » QueryFailed This connector does not support setting table comments
Error:    TestSnowflakeTypeMapping.testTimestamp:306->testTimestamp:344 » QueryFailed Unsupported column type(93):timestampntz
[INFO] 
Error:  Tests run: 313, Failures: 3, Errors: 2, Skipped: 100

https://github.com/trinodb/trino/actions/runs/8154337829/job/22287613018?pr=20895

* fixup! Add Snowflake JDBC Connector * Fix tests * Add the handle for timestamptz back to fix testDataMappingSmokeTest --------- Co-authored-by: Yuya Ebihara <[email protected]>, Erik Anderson <[email protected]>

hongbo-miao · 2024-03-07T01:28:55Z

Wow, congratulations! It's finally merged! Big thanks to @yuuteng and @dprophet and the entire Trino team for all the hard work on this! 🎉 🥳 🎊 🥂

bitsondatadev · 2024-03-07T02:07:56Z

❤️🎉🎊Congratulations team ❄️🐇!! Incredible work and persistence @dprophet, @yuuteng, @ebyhr, @martint, @mosabua, and @findepi.

Special thanks to Bloomberg and ForePaaS for dedicating a large portion of time to get this right. Thanks to Bloomberg for providing the integration testing infrastructure (Snowflake resources) to the Trino Software Foundation.

This massive undertaking wouldn't have been possible without you all!

We started investigating why the first two attempts to create a Snowflake connector failed. It turned out that that each time there was a discussion for how to do integration testing, and both PRs stalled. After seeing this, we noticed @yuuteng had already done a significant amount of work for a third attempt to contribute a Snowflake connector in use at ForePaaS. Not long after I had reached out to Teng to brainstorm a solution, @dprophet reached out to get my thoughts on how to go about contributing Bloomberg's connector. Great, now we have two connector implementations and no solution still.

During the first Trino Contributor Congregation, Erik and I brought up Bloomberg's desire to contribute a Snowflake connector and I articulated the testing issue. Erik said something to the effect of, "Oh I wish I would have known that's the problem, the solution is simple, Bloomberg will provide the TSF a Snowflake account." Done!

Shortly after Erik, Teng, and I discussed the best ways to merge their work, set up the Snowflake accounts for the TSF, and start the arduous process of building a thorough test suite with the help of Yuya, Piotr, Manfred, and Martin.

Despite all of the setbacks and infra issues, everyone persisted. We were almost sure this was going to land late summer of 2023. So much so that the goal was to discuss the release during the July presentation of this connector! Regardless, everyone involved persisted.

What a great outcome and example of open source bringing positive sum value to the world of data and analytics. Y'all are incredible!

Federate them all!

❤️❤️

-Bits

yuuteng · 2024-03-07T11:55:02Z

I'm overjoyed to see the Snowflake connector merged today!🎉 This journey started over two years ago on December 22nd, 2021, and it wouldn't have been possible without the tireless efforts of everyone involved. ❄️🐇

@bitsondatadev, thank you for rescuing this project from limbo and revitalizing it, leading to the incredible collaboration between Bloomberg, Trino community and ForePaaS!

@ebyhr, thank you for your patience and guidance over the past two years, reviewing our code and answering our questions like a wise teacher. You've played a vital role in my growth!

@pgagnon, the first contributor who paved the way! I wouldn't know where to start without your work. Thank you for your initial contribution, and I hope you're proud of seeing it merged today!

@dprophet, my dear partner, it's been two amazing years working with you! We overcame time zones and different holidays while working in different countries and companies. Finally, our "baby" is merged! Thank you for making our work shine ✨. You're a fantastic partner!

@cploujoux and @drappier-charles, my managers, and all my colleagues who cared about this project. Thank you for giving me the opportunity to work on this challenging and rewarding task. Your support and advice have been invaluable!

To all the friends who offered help and reviewed our code: Thank you for your enthusiasm and dedication! Your contributions, big or small, have made a real difference. Although I can't list all your names here, your kindness and support are deeply appreciated.

P.S. Sorry for turning this commit message into a heartfelt essay. The past 2 years and 3 months have been an unforgettable journey. Thanks for reading to here, and see you all on the next adventure!

Best wishes,

❤️❤️

Teng (@yuuteng)

cla-bot bot added the cla-signed label Jun 14, 2023

findepi requested a review from hashhar June 14, 2023 20:44

mosabua changed the title ~~Snowflake connector~~ Add Snowflake connector Jun 14, 2023

bitsondatadev requested a review from ebyhr June 14, 2023 22:21

github-actions bot added docs tests:hive labels Jun 14, 2023

ebyhr requested changes Jun 14, 2023

View reviewed changes

ebyhr reviewed Jun 14, 2023

View reviewed changes

ebyhr reviewed Jun 15, 2023

View reviewed changes

yuuteng reviewed Jul 13, 2023

View reviewed changes

chenjian2664 reviewed Aug 7, 2023

View reviewed changes

github-actions bot added ui Web UI hive Hive connector bigquery BigQuery connector labels Oct 31, 2023

github-actions bot added release-notes jdbc Relates to Trino JDBC driver hudi Hudi connector labels Dec 5, 2023

hashhar mentioned this pull request Feb 22, 2024

Add tests to cover Snowflake connector #5431

Closed

dprophet force-pushed the snowflake branch from 38b0d85 to 9dd8e39 Compare February 26, 2024 13:37

ebyhr removed release-notes jdbc Relates to Trino JDBC driver tests:hive hudi Hudi connector iceberg Iceberg connector delta-lake Delta Lake connector hive Hive connector bigquery BigQuery connector mongodb MongoDB connector labels Feb 28, 2024

lxynov reviewed Feb 28, 2024

View reviewed changes

plugin/trino-snowflake/src/main/java/io/trino/plugin/snowflake/SnowflakeClient.java Show resolved Hide resolved

ebyhr mentioned this pull request Mar 1, 2024

Test: Add Snowflake connector #20895

Closed

dprophet force-pushed the snowflake branch from 9dd8e39 to dafaa5d Compare March 4, 2024 18:37

github-actions bot added the tests:hive label Mar 4, 2024

lxynov reviewed Mar 5, 2024

View reviewed changes

yuuteng and others added 2 commits March 6, 2024 11:34

Fix tests 05/03/2024 (#21)

f1b4d12

* fixup! Add Snowflake JDBC Connector * Fix tests * Add the handle for timestamptz back to fix testDataMappingSmokeTest --------- Co-authored-by: Yuya Ebihara <[email protected]>, Erik Anderson <[email protected]>

Update ci.yml, skip checks if no password

26b51b7

ebyhr merged commit 1cdf55d into trinodb:master Mar 7, 2024
96 of 98 checks passed

github-actions bot added this to the 440 milestone Mar 7, 2024

yuuteng mentioned this pull request Mar 7, 2024

Snowflake connector type mapping needs to be updated and fixed #20977

Closed

colebow mentioned this pull request Mar 7, 2024

Add Trino 440 release notes #20797

Merged

findinpath mentioned this pull request Mar 19, 2024

Add Snowflake connector #1863

Closed

lxynov mentioned this pull request Apr 1, 2024

Improve type mapping for Snowflake Connector #21012

Closed

1 task

		private final Type jsonType;
		private final AggregateFunctionRewriter aggregateFunctionRewriter;

Add Snowflake connector #17909

Add Snowflake connector #17909

Conversation

dprophet commented Jun 14, 2023 • edited by mosabua Loading

Description

ebyhr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebyhr Jun 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebyhr Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebyhr Jun 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bitsondatadev commented Jul 10, 2023

yuuteng commented Jul 10, 2023

yuuteng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuuteng commented Jul 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afranzi commented Sep 22, 2023

ebyhr commented Nov 1, 2023

luisfalconeri commented Nov 23, 2023

hongbo-miao commented Nov 29, 2023 • edited Loading

yuuteng commented Nov 29, 2023

bitsondatadev commented Nov 29, 2023

mosabua commented Nov 29, 2023

bitsondatadev commented Nov 29, 2023

yuuteng commented Dec 4, 2023

ebyhr commented Feb 26, 2024 • edited Loading

lxynov Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dprophet commented Jun 14, 2023 •

edited by mosabua

Loading

ebyhr left a comment •

edited

Loading

ebyhr Jun 14, 2023 •

edited

Loading

ebyhr Dec 8, 2023 •

edited

Loading

ebyhr Jun 15, 2023 •

edited

Loading

hongbo-miao commented Nov 29, 2023 •

edited

Loading

ebyhr commented Feb 26, 2024 •

edited

Loading

lxynov Mar 5, 2024 •

edited

Loading

ebyhr Mar 6, 2024 •

edited

Loading