You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "borislitvak (via GitHub)" <gi...@apache.org> on 2024/04/30 10:30:51 UTC

[PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

borislitvak opened a new pull request, #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2084941773

   Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2268938459

   @davidradl WDYT?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1591898186


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeRowConverter.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.converter.AbstractPostgresCompatibleRowConverter;
+import org.apache.flink.table.types.logical.RowType;
+
+import org.postgresql.jdbc.PgArray;
+
+/**
+ * Runtime converter that responsible to convert between JDBC object and Flink internal object for
+ * Snowflake.
+ */
+@Internal
+public class SnowflakeRowConverter extends AbstractPostgresCompatibleRowConverter<PgArray> {
+
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    public String converterName() {
+        return "Snowflake";
+    }
+
+    // https://docs.snowflake.com/en/sql-reference/intro-summary-data-types
+    public SnowflakeRowConverter(RowType rowType) {

Review Comment:
   @davidradl Could you please take a look at this PR?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1585423831


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeRowConverter.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.converter.AbstractPostgresCompatibleRowConverter;
+import org.apache.flink.table.types.logical.RowType;
+
+import org.postgresql.jdbc.PgArray;
+
+/**
+ * Runtime converter that responsible to convert between JDBC object and Flink internal object for
+ * Snowflake.
+ */
+@Internal
+public class SnowflakeRowConverter extends AbstractPostgresCompatibleRowConverter<PgArray> {
+
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    public String converterName() {
+        return "Snowflake";
+    }
+
+    // https://docs.snowflake.com/en/sql-reference/intro-summary-data-types
+    public SnowflakeRowConverter(RowType rowType) {

Review Comment:
   
   
   
   > I am not sure exactly how Snowflake SQL compares to the other SQLS. It seems to be close to SQL server , which has some TINYINT logic in its converter - it looks like Snowflake supports this type - so you need to add this to the converter.
   > 
   > I also see https://docs.uipath.com/process-mining/automation-suite/2022.10/user-guide/sql-differences-between-snowflake-and-sql-server. Do any of these SQL quirks require converter changes?
   > 
   > I notice that Snowflake SQL uses double quotes, where as Flink SQL uses back ticks , it would be worth testing that quotes work for Snowflake.
   
   https://docs.snowflake.com/en/sql-reference/intro-summary-data-types
   
   This code was tested with Flink, so whatever Flink does should work. I've provided an example in the PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1586816863


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeDialectFactory.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialect;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialectFactory;
+
+/** Factory for {@link SnowflakeDialect}. */
+@Internal
+public class SnowflakeDialectFactory implements JdbcDialectFactory {
+    @Override
+    public boolean acceptsURL(String url) {
+        return url.startsWith("jdbc:snowflake:");
+    }
+
+    @Override
+    public JdbcDialect create() {

Review Comment:
   Docs pushed as well. Please take a look @davidradl 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2514882321

   > @davidradl, I used this repo to try and create a connector myself, however I found that some of the methods(ex createInputSerializer()) are not available on flink 1.19 I have to refactor. Is it possible to resolve some of them in 1.20? And when do you think the new version will be available ? :)
   > 
   > Thank you!
   
   Hi @EcaterinaL , I am not sure what you mean by "Is it possible to resolve some of them in 1.20?" I suggest checking the git history to see when the methods were removed and how callers now get this capability. At the moment it is early days in getting a JDBC connector out that works with 1.20; I initiated a conversation on the dev list to get this tarted. The release manager has not been agreed yet, so I do not have a timescale for you.  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "EcaterinaL (via GitHub)" <gi...@apache.org>.
EcaterinaL commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2514868966

   @davidradl, I used this repo to try and create a connector myself, however I found that some of the methods(ex createInputSerializer()) are not available on flink 1.19 I have to refactor. Is it possible to resolve some of them in 1.20? And when do you think the new version will be available ? :)
   
   Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "EcaterinaL (via GitHub)" <gi...@apache.org>.
EcaterinaL commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2485369191

   Hello all, 
   
   Is there any progress on this? will it be merged, if yes, when do you think this would be?
   
   Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1584685732


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeDialectFactory.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialect;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialectFactory;
+
+/** Factory for {@link SnowflakeDialect}. */
+@Internal
+public class SnowflakeDialectFactory implements JdbcDialectFactory {
+    @Override
+    public boolean acceptsURL(String url) {
+        return url.startsWith("jdbc:snowflake:");
+    }
+
+    @Override
+    public JdbcDialect create() {

Review Comment:
   Please could you add some junits and docs for this pr.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1585425522


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeDialectFactory.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialect;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialectFactory;
+
+/** Factory for {@link SnowflakeDialect}. */
+@Internal
+public class SnowflakeDialectFactory implements JdbcDialectFactory {
+    @Override
+    public boolean acceptsURL(String url) {
+        return url.startsWith("jdbc:snowflake:");
+    }
+
+    @Override
+    public JdbcDialect create() {

Review Comment:
   Will do!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2487685106

   > Hello all,
   > 
   > Is there any progress on this? will it be merged, if yes, when do you think this would be?
   > 
   > Thank you!
   
   Ecaterina, I am still waiting for a code review for a year. Using this in prod with 1.19.1.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1586364059


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeDialectFactory.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialect;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialectFactory;
+
+/** Factory for {@link SnowflakeDialect}. */
+@Internal
+public class SnowflakeDialectFactory implements JdbcDialectFactory {
+    @Override
+    public boolean acceptsURL(String url) {
+        return url.startsWith("jdbc:snowflake:");
+    }
+
+    @Override
+    public JdbcDialect create() {

Review Comment:
   Pushed a unit test, docs are still WIP



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "EcaterinaL (via GitHub)" <gi...@apache.org>.
EcaterinaL commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2514896195

   Thank you @borislitvak, I'll look at the history, good idea. I'm going to keep monitor this thread, for me is important to have this connector ready in coming months. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1584710588


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeRowConverter.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.converter.AbstractPostgresCompatibleRowConverter;
+import org.apache.flink.table.types.logical.RowType;
+
+import org.postgresql.jdbc.PgArray;
+
+/**
+ * Runtime converter that responsible to convert between JDBC object and Flink internal object for
+ * Snowflake.
+ */
+@Internal
+public class SnowflakeRowConverter extends AbstractPostgresCompatibleRowConverter<PgArray> {
+
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    public String converterName() {
+        return "Snowflake";
+    }
+
+    // https://docs.snowflake.com/en/sql-reference/intro-summary-data-types
+    public SnowflakeRowConverter(RowType rowType) {

Review Comment:
   I am not sure exactly how Snowflake SQL compares to the other SQLS. It seems to be close to SQL server , which has some TINYINT logic in its converter - it looks like Snowflake supports this type - so you need to add this to the converter.
   
   I also see [https://docs.uipath.com/process-mining/automation-suite/2022.10/user-guide/sql-differences-between-snowflake-and-sql-server](https://docs.uipath.com/process-mining/automation-suite/2022.10/user-guide/sql-differences-between-snowflake-and-sql-server). Do any of these SQL quirks require converter changes? 
   
   I notice that Snowflake SQL uses double quotes, where as Flink SQL uses back ticks , it would be worth testing that quotes work for Snowflake.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1584710588


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeRowConverter.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.converter.AbstractPostgresCompatibleRowConverter;
+import org.apache.flink.table.types.logical.RowType;
+
+import org.postgresql.jdbc.PgArray;
+
+/**
+ * Runtime converter that responsible to convert between JDBC object and Flink internal object for
+ * Snowflake.
+ */
+@Internal
+public class SnowflakeRowConverter extends AbstractPostgresCompatibleRowConverter<PgArray> {
+
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    public String converterName() {
+        return "Snowflake";
+    }
+
+    // https://docs.snowflake.com/en/sql-reference/intro-summary-data-types
+    public SnowflakeRowConverter(RowType rowType) {

Review Comment:
   I am not sure exactly how Snowflake SQL compares to the other SQLS. It seems to be close to SQL server , which has some TINYINT logic in its converter - it looks like Snowflake supports this.
   
   I also see [https://docs.uipath.com/process-mining/automation-suite/2022.10/user-guide/sql-differences-between-snowflake-and-sql-server](https://docs.uipath.com/process-mining/automation-suite/2022.10/user-guide/sql-differences-between-snowflake-and-sql-server). Do any of these SQL quirks require converter changes? 
   
   I notice that Snowflake SQL uses double quotes, where as Flink SQL uses back ticks , it would be worth testing that quotes work for Snowflake.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-3695581005

   This PR has been closed since it has not had any activity in 120 days. 
   If you feel like this was a mistake, or you would like to continue working on it, 
   please feel free to re-open the PR and ask for a review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1584685732


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeDialectFactory.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialect;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialectFactory;
+
+/** Factory for {@link SnowflakeDialect}. */
+@Internal
+public class SnowflakeDialectFactory implements JdbcDialectFactory {
+    @Override
+    public boolean acceptsURL(String url) {
+        return url.startsWith("jdbc:snowflake:");
+    }
+
+    @Override
+    public JdbcDialect create() {

Review Comment:
   Please could you add some junits for this pr.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#discussion_r1584685732


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/databases/snowflake/dialect/SnowflakeDialectFactory.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.connector.jdbc.databases.snowflake.dialect;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialect;
+import org.apache.flink.connector.jdbc.dialect.JdbcDialectFactory;
+
+/** Factory for {@link SnowflakeDialect}. */
+@Internal
+public class SnowflakeDialectFactory implements JdbcDialectFactory {
+    @Override
+    public boolean acceptsURL(String url) {
+        return url.startsWith("jdbc:snowflake:");
+    }
+
+    @Override
+    public JdbcDialect create() {

Review Comment:
   Please could you add some junits and docs for this pr please.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-3591042042

   This PR is being marked as stale since it has not had any activity in the last 90 days. 
   If you would like to keep this PR alive, please leave a comment asking for a review. 
   If the PR has merge conflicts, update it with the latest from the base branch.
   
   If you are having difficulty finding a reviewer, please reach out to the 
   community, contact details can be found here: https://flink.apache.org/what-is-flink/community/
   
   If this PR is no longer valid or desired, please feel free to close it. 
   If no activity occurs in the next 30 days, it will be automatically closed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "borislitvak (via GitHub)" <gi...@apache.org>.
borislitvak commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2488981196

   I actively use it, yes. You can built from branch.
   
   On Wed, Nov 20, 2024, 17:24 Ecaterina Lazarescu ***@***.***>
   wrote:
   
   > Thank you for your prompt answer!,
   > Do you actively use it?
   > Have you merge it with your master?
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2488874691>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ACKDRNMTPNR7WTWC4LVYVT32BSSRRAVCNFSM6AAAAABG77RBHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBYHA3TINRZGE>
   > .
   > You are receiving this because you authored the thread.Message ID:
   > ***@***.***>
   >
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2514787613

   Hi @borislitvak @EcaterinaL , thanks for the changes - I cannot merge this as I am not a committer. 
   I notice the branch has conflicts.
   I have suggested this be included in the forthcoming JDBC connector for Flink 1.20. 
   
   @snuyanzin is this something you can help with reviewing/merging once the conflicts are resolved please? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2514913661

   > Thank you @davidradl , I'll look at the history, good idea. I'm going to keep monitor this thread, for me is important to have this connector ready in coming months.
   
   no worries - I also suggest that you respond with your need to my dev list post - I proposed that we can get a 1.20 release cut off main - then if your PR is merged this dialect will be included.     


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "EcaterinaL (via GitHub)" <gi...@apache.org>.
EcaterinaL commented on PR #118:
URL: https://github.com/apache/flink-connector-jdbc/pull/118#issuecomment-2488874691

   
   Thank you for your prompt answer!,
   Do you actively use it?
   Have you merge it with your master? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect [flink-connector-jdbc]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #118: [FLINK-33761][Connector/JDBC] Add Snowflake JDBC Dialect
URL: https://github.com/apache/flink-connector-jdbc/pull/118


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org