You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "FangYongs (via GitHub)" <gi...@apache.org> on 2023/04/06 03:37:00 UTC

[GitHub] [flink] FangYongs opened a new pull request, #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

FangYongs opened a new pull request, #22360:
URL: https://github.com/apache/flink/pull/22360

   ## What is the purpose of the change
   
   This PR aims to convert string value in `StatementResult` to different value for `ResultSet`
   
   
   ## Brief change log
   
     - Added `DataConverter` api
     - Added `DefaultDataConverter` and `StringDataConverter`
   
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
     - Added test `FlinkResultSetTest.testStringResultSetPrimitiveData`
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (yes / no) no
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / no) no
     - The serializers: (yes / no / don't know) no
     - The runtime per-record code paths (performance sensitive): (yes / no / don't know) no
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know) no
     - The S3 file system connector: (yes / no / don't know) no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (yes / no) no
     - If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
   


-- 
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


[GitHub] [flink] libenchao closed pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao closed pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result
URL: https://github.com/apache/flink/pull/22360


-- 
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


[GitHub] [flink] FangYongs commented on pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on PR #22360:
URL: https://github.com/apache/flink/pull/22360#issuecomment-1507869543

   Thanks @libenchao I have updated 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


[GitHub] [flink] FangYongs commented on pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on PR #22360:
URL: https://github.com/apache/flink/pull/22360#issuecomment-1498448558

   Hi @libenchao Please help to review this PR when you're free, thanks


-- 
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


[GitHub] [flink] FangYongs commented on pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on PR #22360:
URL: https://github.com/apache/flink/pull/22360#issuecomment-1512288840

   Thanks @libenchao DONE
   


-- 
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


[GitHub] [flink] FangYongs commented on a diff in pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on code in PR #22360:
URL: https://github.com/apache/flink/pull/22360#discussion_r1164814536


##########
flink-table/flink-sql-jdbc-driver/src/main/java/org/apache/flink/table/jdbc/utils/StringDataConverter.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.table.jdbc.utils;
+
+import org.apache.flink.table.data.RowData;
+import org.apache.flink.table.data.StringData;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Timestamp;
+import java.util.Map;
+
+/** Converter string value to different value. */
+public class StringDataConverter implements DataConverter {

Review Comment:
   Currently SqlGateway will convert all data to string in StatementResult, we need to convert to different value in FlinkResultSet when we get data from StatementResult



-- 
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


[GitHub] [flink] flinkbot commented on pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22360:
URL: https://github.com/apache/flink/pull/22360#issuecomment-1498449680

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "dcbaa06d17c31c7d5f9eaff064ea3849fd1b1670",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "dcbaa06d17c31c7d5f9eaff064ea3849fd1b1670",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * dcbaa06d17c31c7d5f9eaff064ea3849fd1b1670 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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


[GitHub] [flink] libenchao commented on a diff in pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on code in PR #22360:
URL: https://github.com/apache/flink/pull/22360#discussion_r1168600675


##########
flink-table/flink-sql-jdbc-driver/src/main/java/org/apache/flink/table/jdbc/utils/DefaultDataConverter.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.table.jdbc.utils;
+
+import org.apache.flink.table.data.RowData;
+import org.apache.flink.table.data.StringData;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Timestamp;
+import java.util.Map;
+
+/** Default data converter for result set. */
+public class DefaultDataConverter implements DataConverter {
+    public static final DataConverter CONVERTER = new DefaultDataConverter();
+
+    private DefaultDataConverter() {}
+
+    @Override
+    public boolean getBoolean(RowData rowData, int pos) {
+        return rowData.getBoolean(pos);
+    }
+
+    @Override
+    public byte getByte(RowData rowData, int pos) {
+        return rowData.getByte(pos);
+    }
+
+    @Override
+    public short getShort(RowData rowData, int pos) {
+        return rowData.getShort(pos);
+    }
+
+    @Override
+    public int getInt(RowData rowData, int pos) {
+        return rowData.getInt(pos);

Review Comment:
   `DefaultDataConverter` also needs to handle `null`?



-- 
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


[GitHub] [flink] libenchao commented on a diff in pull request #22360: [FLINK-31741][jdbc-driver] Support data converter for value in statement result

Posted by "libenchao (via GitHub)" <gi...@apache.org>.
libenchao commented on code in PR #22360:
URL: https://github.com/apache/flink/pull/22360#discussion_r1164056558


##########
flink-table/flink-sql-jdbc-driver/src/main/java/org/apache/flink/table/jdbc/utils/StringDataConverter.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.table.jdbc.utils;
+
+import org.apache.flink.table.data.RowData;
+import org.apache.flink.table.data.StringData;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Timestamp;
+import java.util.Map;
+
+/** Converter string value to different value. */
+public class StringDataConverter implements DataConverter {

Review Comment:
   I'm not sure in what cases we'll use this class, could you elaborate a little more?



##########
flink-table/flink-sql-jdbc-driver/src/main/java/org/apache/flink/table/jdbc/FlinkResultSet.java:
##########
@@ -184,7 +185,7 @@ public int getInt(int columnIndex) throws SQLException {
         checkValidRow();
         checkValidColumn(columnIndex);
         try {
-            return currentRow.getInt(columnIndex - 1);

Review Comment:
   According to the comment of `java.sql.ResultSet#getInt`, we need to adjust `null` to the corresponding value. This is the same for other primitive types.
   ```java
        * @return the column value; if the value is SQL <code>NULL</code>, the
        * value returned is <code>0</code>
   ```



-- 
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