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 2021/07/02 00:51:48 UTC

[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7117: Add timestamp datatype support in JDBC

Jackie-Jiang commented on a change in pull request #7117:
URL: https://github.com/apache/incubator-pinot/pull/7117#discussion_r662666988



##########
File path: pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotPreparedStatement.java
##########
@@ -33,20 +33,25 @@
 public class PinotPreparedStatement extends AbstractBasePreparedStatement {
 
   private static final String QUERY_FORMAT = "sql";
+  public static final String LIMIT_STATEMENT = "LIMIT";

Review comment:
       Doesn't need to be public

##########
File path: pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotStatement.java
##########
@@ -27,11 +27,13 @@
 public class PinotStatement extends AbstractBaseStatement {

Review comment:
       Same for this class

##########
File path: pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotPreparedStatement.java
##########
@@ -33,20 +33,25 @@
 public class PinotPreparedStatement extends AbstractBasePreparedStatement {
 
   private static final String QUERY_FORMAT = "sql";
+  public static final String LIMIT_STATEMENT = "LIMIT";
   private Connection _connection;
   private org.apache.pinot.client.Connection _session;
   private ResultSetGroup _resultSetGroup;
   private PreparedStatement _preparedStatement;
   private String _query;
   private boolean _closed;
   private ResultSet _resultSet;
+  private Integer _maxRows = Integer.MAX_VALUE;

Review comment:
       primitive `int`

##########
File path: pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotPreparedStatement.java
##########
@@ -33,20 +33,25 @@
 public class PinotPreparedStatement extends AbstractBasePreparedStatement {
 
   private static final String QUERY_FORMAT = "sql";
+  public static final String LIMIT_STATEMENT = "LIMIT";
   private Connection _connection;
   private org.apache.pinot.client.Connection _session;
   private ResultSetGroup _resultSetGroup;
   private PreparedStatement _preparedStatement;
   private String _query;
   private boolean _closed;
   private ResultSet _resultSet;
+  private Integer _maxRows = Integer.MAX_VALUE;
 
   public PinotPreparedStatement(PinotConnection connection, String query) {
     _connection = connection;
     _session = connection.getSession();
-    _query = query;
     _closed = false;
-    _preparedStatement = new PreparedStatement(_session, new Request(QUERY_FORMAT, query));
+    _query = query;
+    if(!_query.contains(LIMIT_STATEMENT)) {

Review comment:
       I would suggest making limit statement ` limit ` (or using regex with whitespaces on both side) to avoid matching column name such as `SOME_LIMIT`. Also, we should ignore the case while matching them.
   Ideally we should compile the query, but that might be too much overhead




-- 
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: commits-unsubscribe@pinot.apache.org

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