You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/07/09 13:54:13 UTC

[GitHub] [incubator-pinot] KKcorps opened a new pull request #5673: Fetch Columns and DataType from ResultSet

KKcorps opened a new pull request #5673:
URL: https://github.com/apache/incubator-pinot/pull/5673


   The patch is needed to provide ResultSet metadata to JDBC driver.
   The driver requires a column data type in response and currently there is not way to fetch that.


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] KKcorps commented on a change in pull request #5673: Fetch Columns and DataType from ResultSet

Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #5673:
URL: https://github.com/apache/incubator-pinot/pull/5673#discussion_r452366297



##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ResultTableResultSet.java
##########
@@ -61,6 +63,33 @@ public String getString(int rowIndex, int columnIndex) {
     }
   }
 
+  public List<String> getAllColumns() {
+    List<String> columns = new ArrayList<>();
+    if (_columnNamesArray == null) {
+      return columns;
+    }
+
+    for (JsonNode column : _columnNamesArray) {
+      columns.add(column.textValue());
+    }
+
+    return columns;
+  }
+
+
+  public List<String> getAllColumnsDataTypes() {
+    List<String> columnDataTypes = new ArrayList<>();
+    if (_columnDataTypesArray == null) {

Review comment:
       It is just an array of strings so does it make sense to create an object for it?
   We are anyways not doing it anywhere in java client and using JsonNode.
   




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5673: Fetch Columns and DataType from ResultSet

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #5673:
URL: https://github.com/apache/incubator-pinot/pull/5673


   


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] KKcorps commented on a change in pull request #5673: Fetch Columns and DataType from ResultSet

Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #5673:
URL: https://github.com/apache/incubator-pinot/pull/5673#discussion_r452366492



##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ResultTableResultSet.java
##########
@@ -61,6 +63,33 @@ public String getString(int rowIndex, int columnIndex) {
     }
   }
 
+  public List<String> getAllColumns() {
+    List<String> columns = new ArrayList<>();
+    if (_columnNamesArray == null) {
+      return columns;
+    }
+
+    for (JsonNode column : _columnNamesArray) {
+      columns.add(column.textValue());
+    }
+
+    return columns;
+  }
+
+
+  public List<String> getAllColumnsDataTypes() {
+    List<String> columnDataTypes = new ArrayList<>();
+    if (_columnDataTypesArray == null) {

Review comment:
       I can create a seperate PR though for replacing all instances of JsonNode with proper classes.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] KKcorps commented on pull request #5673: Fetch Columns and DataType from ResultSet

Posted by GitBox <gi...@apache.org>.
KKcorps commented on pull request #5673:
URL: https://github.com/apache/incubator-pinot/pull/5673#issuecomment-656367342


   @kishoreg can we merge it now? 
   


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #5673: Fetch Columns and DataType from ResultSet

Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #5673:
URL: https://github.com/apache/incubator-pinot/pull/5673#discussion_r452322783



##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ResultTableResultSet.java
##########
@@ -61,6 +63,33 @@ public String getString(int rowIndex, int columnIndex) {
     }
   }
 
+  public List<String> getAllColumns() {
+    List<String> columns = new ArrayList<>();
+    if (_columnNamesArray == null) {
+      return columns;
+    }
+
+    for (JsonNode column : _columnNamesArray) {
+      columns.add(column.textValue());
+    }
+
+    return columns;
+  }
+
+
+  public List<String> getAllColumnsDataTypes() {
+    List<String> columnDataTypes = new ArrayList<>();
+    if (_columnDataTypesArray == null) {

Review comment:
       Would it be better to model `  private final JsonNode _columnDataTypesArray;` as a Schema object?




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org