zabetak commented on code in PR #6149:
URL: https://github.com/apache/hive/pull/6149#discussion_r2459926863


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java:
##########
@@ -3203,6 +3204,10 @@ private void getStatsTableListResult(
   }
 
   public void lockDbTable(String tableName) throws MetaException {
+    if (!ALLOWED_TABLES_TO_LOCK.contains(tableName)) {
+      throw new MetaException("Error while locking table " + tableName);

Review Comment:
   Let's add a test case for the exception if its easy/possible.



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java:
##########
@@ -3269,16 +3281,21 @@ public List<Void> run(List<String> input) throws 
Exception {
             input, Collections.emptyList(), -1);
         if (!partitionIds.isEmpty()) {
           String deleteSql = "delete from " + PART_COL_STATS + " where 
\"PART_ID\" in ( " + getIdListForIn(partitionIds) + ")";
+          List<Object> params = new ArrayList<>(colNames == null ? 1 : 
colNames.size() + 1);
+
           if (colNames != null && !colNames.isEmpty()) {
-            deleteSql += " and \"COLUMN_NAME\" in (" + 
colNames.stream().map(col -> "'" + col + "'").collect(Collectors.joining(",")) 
+ ")";
+            deleteSql += " and \"COLUMN_NAME\" in (" + 
makeParams(colNames.size()) + ")";
+            params.addAll(colNames);
           }
+
           if (engine != null) {
-            deleteSql += " and \"ENGINE\" = '" + engine + "'";
+            deleteSql += " and \"ENGINE\" = ?";
+            params.add(engine);
           }
-          try {
-            executeNoResult(deleteSql);
-          } catch (SQLException e) {
-            LOG.warn("Error removing partition column stats. ", e);
+
+          try (QueryWrapper queryParams = new 
QueryWrapper(pm.newQuery("javax.jdo.query.SQL", deleteSql))) {
+            executeWithArray(queryParams.getInnerQuery(), params.toArray(), 
deleteSql);
+          } catch (MetaException e) {

Review Comment:
   Catching a MetaException and rethrowing it as a MetaException seems 
redundant and will make the stacktrace harder to follow. Since the method 
already throws `MetaException` can we simply remove the catch block.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to