You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/11/03 13:49:56 UTC

[GitHub] [hive] vnhive opened a new pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

vnhive opened a new pull request #1640:
URL: https://github.com/apache/hive/pull/1640


   …en the number of partitions is greater than 32767 (signed int)
   
   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
     2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
     3. Ensure you have added or run the appropriate tests for your PR: 
     4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX:  Your PR title ...'.
     5. Be sure to keep the PR description updated to reflect all changes.
     6. Please write your PR title to summarize what this PR proposes.
     7. If possible, provide a concise example to reproduce the issue for a faster review.
   
   -->
   
   ### What changes were proposed in this pull request?
   The implementation of getPartitionIdsViaSqlFilter should be extended to account for the limit of 32767
   on the number of parameters to the underlying JDBC driver.
   
   
   ### Why are the changes needed?
   The table partition operations - create, drop, select access the underlying relation database using JDO,
   which internally routes the operations through the JDBC driver. Most of the underlying JDBC driver
   implementations place a limit on the number of parameters that can be passed through a statement
   implementation. These limits should be taken into account when querying the underlying metastore.
   
   
   
   ### Does this PR introduce _any_ user-facing change?
   The patch will ensure that if more than 32767 operations are being operated on (added, dropped or queried)
   at the same time, it will not fail.
   
   
   ### How was this patch tested?
   1. The patch was verified by attaching the debugger and verifying the code flow
   2. The existing metastore tests passed.
   


----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vnhive commented on a change in pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
vnhive commented on a change in pull request #1640:
URL: https://github.com/apache/hive/pull/1640#discussion_r521273558



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""

Review comment:
       Will do ! Will make the change and resubmit.




----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] closed pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #1640:
URL: https://github.com/apache/hive/pull/1640


   


----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vnhive commented on a change in pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
vnhive commented on a change in pull request #1640:
URL: https://github.com/apache/hive/pull/1640#discussion_r521273788



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+

Review comment:
       Will do ! Will make the change and resubmit.




----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vnhive commented on a change in pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
vnhive commented on a change in pull request #1640:
URL: https://github.com/apache/hive/pull/1640#discussion_r521272977



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;

Review comment:
       1. I am OK with this. Will do this and resubmit the patch.
   
   > 2. But do we really know that it works? We dont have a test case for this that tests across all databases nor have we 
   > tested this, afaik.
   
   Agreed. I will look into creating a test. Insofar as the issue itself is concerned they seem to be documented limitations of the JDBC driver and the solution is to break the original query into multiple queries. Please do take a look at - https://www.postgresql.org/message-id/16832734.post%40talk.nabble.com
   
   > We dont have a test case for this that tests across all databases nor have we tested this, afaik.
   
   Agreed. I can work on this.
   
   > Also we are starting to support a pluggable model for DB support. So users can use HMS with other DBs as well for 
   > example DB2 (all they have to do is provide the schema scripts). So we don't know what their limits would be.
   > Would it make sense to use a smaller number? what is the downside of doing so?
   
   Thank you for explaining about the pluggable model for DB support. We can consider a smaller size for the parameters that are passed to the query.
   
   The downside of reducing the number of parameters that are passed would be that it can potentially result in increases the number of iterations required to get the result of the query for the same number of parameters.
   
   for example if there are 30000 parameters, with a size of 32767, it can be executed in a single query. But if we set the size to 1000, it will result in 30000/1000 = 30 iterations.
   
   This will result in increased latency.
   
   It is important to note that this part of the code is executed for all DDL, DML and queries that involved partitioned tables. Hence it has potential to increase the latency across the spectrum.
   




----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vnhive commented on a change in pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
vnhive commented on a change in pull request #1640:
URL: https://github.com/apache/hive/pull/1640#discussion_r521280040



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+
+
+    // The first three parameters in the query are the table, database and the catalog names. This leaves us
+    // JDBC_STMT_PARAM_LIMIT - 3 parameters that can be passed to the underlying JDBC driver. If the number of
+    // input parameters is greater than this limit, we will have to process them in multiple iterations.
+    if (parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3)) {
+      // Find the number of iterations required from the number of input parameters and sets of 32764 parameters
+      // that can be created from it.
+      iterations = parametersToProcess / (JDBC_STMT_PARAM_LIMIT - 3);
+      iterations += ((parametersToProcess % (JDBC_STMT_PARAM_LIMIT - 3) != 0) ? 1 : 0);

Review comment:
       I used his code to distinguish the case when the number of parameters is a multiple of JDBC_STMT_PARAM_LIMIT - 3, vs when it is not.
   
   For example,
   
   when parametersToProcess = 32764 * 2 => iterations = 2
   
   However 
   
   when parametersToProcess = 32764 * 2 + 11 => iterations = 3
   
   32764 parameters in the first iteration
   32764 parameters in the second iteration
   11 parameters in the third iteration




----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] commented on pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1640:
URL: https://github.com/apache/hive/pull/1640#issuecomment-767209581


   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] closed pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #1640:
URL: https://github.com/apache/hive/pull/1640


   


----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] github-actions[bot] commented on pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1640:
URL: https://github.com/apache/hive/pull/1640#issuecomment-767209581


   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vnhive commented on a change in pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
vnhive commented on a change in pull request #1640:
URL: https://github.com/apache/hive/pull/1640#discussion_r521280213



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+
+
+    // The first three parameters in the query are the table, database and the catalog names. This leaves us
+    // JDBC_STMT_PARAM_LIMIT - 3 parameters that can be passed to the underlying JDBC driver. If the number of
+    // input parameters is greater than this limit, we will have to process them in multiple iterations.
+    if (parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3)) {
+      // Find the number of iterations required from the number of input parameters and sets of 32764 parameters
+      // that can be created from it.
+      iterations = parametersToProcess / (JDBC_STMT_PARAM_LIMIT - 3);
+      iterations += ((parametersToProcess % (JDBC_STMT_PARAM_LIMIT - 3) != 0) ? 1 : 0);
     }
 
-    List<Long> result = new ArrayList<>(sqlResult.size());
-    for (Object fields : sqlResult) {
-      result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));
+    for (int i = 0 ; i < iterations; i++) {
+      // The number of filter parameters that can be processed in each iteration has to be lesser than or equal to
+      // JDBC_STMT_PARAM_LIMIT - 3
+      int filterParamSize = parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3) ? (JDBC_STMT_PARAM_LIMIT - 3) : parametersToProcess;
+
+      Object[] params = new Object[filterParamSize + 3];
+      params[0] = tblNameLcase;
+      params[1] = dbNameLcase;
+      params[2] = catNameLcase;
+
+      // Reduce by the number of parameters that have already been processed.
+      parametersToProcess -= (JDBC_STMT_PARAM_LIMIT - 3);
+
+      for (int j = 0; j < filterParamSize; j++) {
+        // The current set of parameters have to be fetched from the correct offset in the input parameters set.
+        params[j + 3] = paramsForFilter.get(((JDBC_STMT_PARAM_LIMIT - 3) * i) + j);
+      }
+
+      long start = doTrace ? System.nanoTime() : 0;
+      Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
+      List<Object> sqlResult = MetastoreDirectSqlUtils.executeWithArray(query, params, queryText, ((max == null) ? -1 : max.intValue()));
+      long queryTime = doTrace ? System.nanoTime() : 0;
+      MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
+
+      for (Object fields : sqlResult) {
+        if (max != null && max != -1) {
+          if (max > 0) {
+            max--;
+          } else {
+            break;
+          }
+        }
+        result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));
+      }
+

Review comment:
       Will do ! Will make the change and resubmit.




----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vnhive commented on a change in pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
vnhive commented on a change in pull request #1640:
URL: https://github.com/apache/hive/pull/1640#discussion_r521282589



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+
+
+    // The first three parameters in the query are the table, database and the catalog names. This leaves us
+    // JDBC_STMT_PARAM_LIMIT - 3 parameters that can be passed to the underlying JDBC driver. If the number of
+    // input parameters is greater than this limit, we will have to process them in multiple iterations.
+    if (parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3)) {
+      // Find the number of iterations required from the number of input parameters and sets of 32764 parameters
+      // that can be created from it.
+      iterations = parametersToProcess / (JDBC_STMT_PARAM_LIMIT - 3);
+      iterations += ((parametersToProcess % (JDBC_STMT_PARAM_LIMIT - 3) != 0) ? 1 : 0);
     }
 
-    List<Long> result = new ArrayList<>(sqlResult.size());
-    for (Object fields : sqlResult) {
-      result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));
+    for (int i = 0 ; i < iterations; i++) {
+      // The number of filter parameters that can be processed in each iteration has to be lesser than or equal to
+      // JDBC_STMT_PARAM_LIMIT - 3
+      int filterParamSize = parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3) ? (JDBC_STMT_PARAM_LIMIT - 3) : parametersToProcess;
+
+      Object[] params = new Object[filterParamSize + 3];
+      params[0] = tblNameLcase;
+      params[1] = dbNameLcase;
+      params[2] = catNameLcase;
+
+      // Reduce by the number of parameters that have already been processed.
+      parametersToProcess -= (JDBC_STMT_PARAM_LIMIT - 3);
+
+      for (int j = 0; j < filterParamSize; j++) {
+        // The current set of parameters have to be fetched from the correct offset in the input parameters set.
+        params[j + 3] = paramsForFilter.get(((JDBC_STMT_PARAM_LIMIT - 3) * i) + j);
+      }
+
+      long start = doTrace ? System.nanoTime() : 0;
+      Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
+      List<Object> sqlResult = MetastoreDirectSqlUtils.executeWithArray(query, params, queryText, ((max == null) ? -1 : max.intValue()));
+      long queryTime = doTrace ? System.nanoTime() : 0;
+      MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
+
+      for (Object fields : sqlResult) {
+        if (max != null && max != -1) {
+          if (max > 0) {
+            max--;
+          } else {
+            break;
+          }
+        }
+        result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));

Review comment:
       This is the query that is executed,
   
   select "PARTITIONS"."PART_ID" from "PARTITIONS"  inner join "TBLS" on "PARTITIONS"."TBL_ID" = "TBLS"."TBL_ID"     and "TBLS"."TBL_NAME" = ?   inner join "DBS" on "TBLS"."DB_ID" = "DBS"."DB_ID"      and "DBS"."NAME" = ? inner join "PARTITION_KEY_VALS" "FILTER0" on "FILTER0"."PART_ID" = "PARTITIONS"."PART_ID" and "FILTER0"."INTEGER_IDX" = 0 where "DBS"."CTLG_NAME" = ?  and ( (("FILTER0"."PART_KEY_VAL" = ?) or  (("FILTER0"."PART_KEY_VAL" = ?) or  (("FILTER0"."PART_KEY_VAL" = ?) or ("FILTER0"."PART_KEY_VAL" = ?)) ) ) )
   
   We pass in unique values to PART_KEY_VAL, hence this ideally should not result in duplicates.




----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] nrg4878 commented on a change in pull request #1640: HIVE-24343: Table partition operations (create, drop, select) fail wh…

Posted by GitBox <gi...@apache.org>.
nrg4878 commented on a change in pull request #1640:
URL: https://github.com/apache/hive/pull/1640#discussion_r519888844



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""

Review comment:
       re-indent this code to original spacing

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;

Review comment:
       2 thoughts on this
   1) Can we move this outside this method to the top of the class? 
   2) I understand that based on the research all DBs support atleast 32767 parameters. But do we really know that it works? We dont have a test case for this that tests across all databases nor have we tested this, afaik. Also we are starting to support a pluggable model for DB support. So users can use HMS with other DBs as well for example DB2 (all they have to do is provide the schema scripts). So we don't know what their limits would be.
   Would it make sense to use a smaller number? what is the downside of doing so? 

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+

Review comment:
       remove extra blank lines. max of 1 blank line between code blocks.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+
+
+    // The first three parameters in the query are the table, database and the catalog names. This leaves us
+    // JDBC_STMT_PARAM_LIMIT - 3 parameters that can be passed to the underlying JDBC driver. If the number of
+    // input parameters is greater than this limit, we will have to process them in multiple iterations.
+    if (parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3)) {
+      // Find the number of iterations required from the number of input parameters and sets of 32764 parameters
+      // that can be created from it.
+      iterations = parametersToProcess / (JDBC_STMT_PARAM_LIMIT - 3);
+      iterations += ((parametersToProcess % (JDBC_STMT_PARAM_LIMIT - 3) != 0) ? 1 : 0);

Review comment:
       we should be able to merge these 2 statements. iterations is initialized to 1. Within this conditional block, if we just added the result of integer division to the value of iterations, should be good right? This code is only executed when the params is greater than the limit.

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+
+
+    // The first three parameters in the query are the table, database and the catalog names. This leaves us
+    // JDBC_STMT_PARAM_LIMIT - 3 parameters that can be passed to the underlying JDBC driver. If the number of
+    // input parameters is greater than this limit, we will have to process them in multiple iterations.
+    if (parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3)) {
+      // Find the number of iterations required from the number of input parameters and sets of 32764 parameters
+      // that can be created from it.
+      iterations = parametersToProcess / (JDBC_STMT_PARAM_LIMIT - 3);
+      iterations += ((parametersToProcess % (JDBC_STMT_PARAM_LIMIT - 3) != 0) ? 1 : 0);
     }
 
-    List<Long> result = new ArrayList<>(sqlResult.size());
-    for (Object fields : sqlResult) {
-      result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));
+    for (int i = 0 ; i < iterations; i++) {
+      // The number of filter parameters that can be processed in each iteration has to be lesser than or equal to
+      // JDBC_STMT_PARAM_LIMIT - 3
+      int filterParamSize = parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3) ? (JDBC_STMT_PARAM_LIMIT - 3) : parametersToProcess;
+
+      Object[] params = new Object[filterParamSize + 3];
+      params[0] = tblNameLcase;
+      params[1] = dbNameLcase;
+      params[2] = catNameLcase;
+
+      // Reduce by the number of parameters that have already been processed.
+      parametersToProcess -= (JDBC_STMT_PARAM_LIMIT - 3);
+
+      for (int j = 0; j < filterParamSize; j++) {
+        // The current set of parameters have to be fetched from the correct offset in the input parameters set.
+        params[j + 3] = paramsForFilter.get(((JDBC_STMT_PARAM_LIMIT - 3) * i) + j);
+      }
+
+      long start = doTrace ? System.nanoTime() : 0;
+      Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
+      List<Object> sqlResult = MetastoreDirectSqlUtils.executeWithArray(query, params, queryText, ((max == null) ? -1 : max.intValue()));
+      long queryTime = doTrace ? System.nanoTime() : 0;
+      MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
+
+      for (Object fields : sqlResult) {
+        if (max != null && max != -1) {
+          if (max > 0) {
+            max--;
+          } else {
+            break;
+          }
+        }
+        result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));

Review comment:
       I dont think so but I am asking it anyways. Is it possible to have duplicates across sub-queries?

##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
##########
@@ -870,44 +870,88 @@ private boolean isViewTable(String catName, String dbName, String tblName) throw
       List<? extends Object> paramsForFilter, List<String> joinsForFilter, Integer max)
       throws MetaException {
     boolean doTrace = LOG.isDebugEnabled();
+
+    // The JDBC client driver implementations of Derby, PostgreSQL, MySQL, MariaDB and Oracle send the number of
+    // parameters being set in a executed statement as a 2 byte signed integer to the servers. This indirectly
+    // sets s limit on the number of parameters that can be set to 32767. This is also the number of parameters
+    // that can be passed to the JDO executeArray call.
+    final int JDBC_STMT_PARAM_LIMIT = 32767;
+
     final String dbNameLcase = dbName.toLowerCase();
     final String tblNameLcase = tblName.toLowerCase();
     final String catNameLcase = normalizeSpace(catName).toLowerCase();
 
     // We have to be mindful of order during filtering if we are not returning all partitions.
+    // If the number of parameters is larger than 32767 and the query is split into multiple
+    // parts, the order by would work within each query part.
     String orderForFilter = (max != null) ? " order by \"PART_NAME\" asc" : "";
 
     String queryText =
-        "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
-      + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
-      + "    and " + TBLS + ".\"TBL_NAME\" = ? "
-      + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
-      + "     and " + DBS + ".\"NAME\" = ? "
-      + join(joinsForFilter, ' ')
-      + " where " + DBS + ".\"CTLG_NAME\" = ? "
-      + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
-    Object[] params = new Object[paramsForFilter.size() + 3];
-    params[0] = tblNameLcase;
-    params[1] = dbNameLcase;
-    params[2] = catNameLcase;
-    for (int i = 0; i < paramsForFilter.size(); ++i) {
-      params[i + 3] = paramsForFilter.get(i);
-    }
+            "select " + PARTITIONS + ".\"PART_ID\" from " + PARTITIONS + ""
+                    + "  inner join " + TBLS + " on " + PARTITIONS + ".\"TBL_ID\" = " + TBLS + ".\"TBL_ID\" "
+                    + "    and " + TBLS + ".\"TBL_NAME\" = ? "
+                    + "  inner join " + DBS + " on " + TBLS + ".\"DB_ID\" = " + DBS + ".\"DB_ID\" "
+                    + "     and " + DBS + ".\"NAME\" = ? "
+                    + join(joinsForFilter, ' ')
+                    + " where " + DBS + ".\"CTLG_NAME\" = ? "
+                    + (StringUtils.isBlank(sqlFilter) ? "" : (" and " + sqlFilter)) + orderForFilter;
 
-    long start = doTrace ? System.nanoTime() : 0;
-    Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
-    List<Object> sqlResult = executeWithArray(query, params, queryText, ((max == null)  ? -1 : max.intValue()));
-    long queryTime = doTrace ? System.nanoTime() : 0;
-    MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-    if (sqlResult.isEmpty()) {
-      return Collections.emptyList(); // no partitions, bail early.
+    int parametersToProcess = paramsForFilter.size();
+
+    List<Long> result = new ArrayList<>();
+
+    // If this method has been called by default we need to query the tables once. Hence set the minimum numbner of
+    // iterations to 1.
+    int iterations = 1;
+
+
+    // The first three parameters in the query are the table, database and the catalog names. This leaves us
+    // JDBC_STMT_PARAM_LIMIT - 3 parameters that can be passed to the underlying JDBC driver. If the number of
+    // input parameters is greater than this limit, we will have to process them in multiple iterations.
+    if (parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3)) {
+      // Find the number of iterations required from the number of input parameters and sets of 32764 parameters
+      // that can be created from it.
+      iterations = parametersToProcess / (JDBC_STMT_PARAM_LIMIT - 3);
+      iterations += ((parametersToProcess % (JDBC_STMT_PARAM_LIMIT - 3) != 0) ? 1 : 0);
     }
 
-    List<Long> result = new ArrayList<>(sqlResult.size());
-    for (Object fields : sqlResult) {
-      result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));
+    for (int i = 0 ; i < iterations; i++) {
+      // The number of filter parameters that can be processed in each iteration has to be lesser than or equal to
+      // JDBC_STMT_PARAM_LIMIT - 3
+      int filterParamSize = parametersToProcess > (JDBC_STMT_PARAM_LIMIT - 3) ? (JDBC_STMT_PARAM_LIMIT - 3) : parametersToProcess;
+
+      Object[] params = new Object[filterParamSize + 3];
+      params[0] = tblNameLcase;
+      params[1] = dbNameLcase;
+      params[2] = catNameLcase;
+
+      // Reduce by the number of parameters that have already been processed.
+      parametersToProcess -= (JDBC_STMT_PARAM_LIMIT - 3);
+
+      for (int j = 0; j < filterParamSize; j++) {
+        // The current set of parameters have to be fetched from the correct offset in the input parameters set.
+        params[j + 3] = paramsForFilter.get(((JDBC_STMT_PARAM_LIMIT - 3) * i) + j);
+      }
+
+      long start = doTrace ? System.nanoTime() : 0;
+      Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
+      List<Object> sqlResult = MetastoreDirectSqlUtils.executeWithArray(query, params, queryText, ((max == null) ? -1 : max.intValue()));
+      long queryTime = doTrace ? System.nanoTime() : 0;
+      MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
+
+      for (Object fields : sqlResult) {
+        if (max != null && max != -1) {
+          if (max > 0) {
+            max--;
+          } else {
+            break;
+          }
+        }
+        result.add(MetastoreDirectSqlUtils.extractSqlLong(fields));
+      }
+

Review comment:
       remove blank line




----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org