revert null part_vals check in client

apache · May 14, 2024 · 93058a7 · 93058a7
1 parent 44ba9f6
commit 93058a7
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 58 deletions.
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/PartitionTree.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/PartitionTree.java
@@ -122,8 +122,8 @@ List<Partition> addPartitions(List<Partition> partitions, boolean ifNotExists)
    * {@link MetaStoreUtils#getPvals(List, Map)}
    */
   List<Partition> getPartitionsByPartitionVals(List<String> partialPartVals) throws MetaException {
-    if (MetaStoreUtils.arePartValsEmpty(partialPartVals)) {
-      return new ArrayList<>(parts.values());
+    if (partialPartVals == null || partialPartVals.isEmpty()) {
+      throw new MetaException("Partition partial vals cannot be null or empty");
     }
     String partNameMatcher = makePartNameMatcher(tTable, partialPartVals, ".*");
     List<Partition> matchedPartitions = new ArrayList<>();

diff --git a/.../metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java b/.../metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
@@ -2245,8 +2245,8 @@ public List<Partition> listPartitions(String db_name, String tbl_name,
   public List<Partition> listPartitions(String catName, String db_name, String tbl_name,
                                         List<String> part_vals, int max_parts) throws TException {
     // TODO should we add capabilities here as well as it returns Partition objects
-    if (db_name == null || tbl_name == null) {
-      throw new MetaException("Database name/Table name should not be null");
+    if (db_name == null || tbl_name == null || part_vals == null) {
+      throw new MetaException("Database name/Table name/partition values should not be null");
     }
     GetPartitionsPsWithAuthRequest req = createThriftPartitionsReq(GetPartitionsPsWithAuthRequest.class, conf);
     req.setDbName(db_name);
@@ -2361,8 +2361,8 @@ public List<Partition> listPartitionsWithAuthInfo(String catName, String dbName,
   protected List<Partition> listPartitionsWithAuthInfoInternal(String catName, String dbName, String tableName,
       List<String> partialPvals, int maxParts, String userName, List<String> groupNames)
       throws TException {
-    if (dbName == null || tableName == null) {
-      throw new MetaException("Database name/Table name should not be null");
+    if (dbName == null || tableName == null || partialPvals == null) {
+      throw new MetaException("Database name/Table name/partition values should not be null");
     }
     GetPartitionsPsWithAuthRequest req = createThriftPartitionsReq(GetPartitionsPsWithAuthRequest.class, conf);
     req.setTblName(tableName);

diff --git a/...e/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java b/...e/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
@@ -1291,7 +1291,9 @@ public int getNumPartitionsViaSqlFilter(SqlFilterForPushdown filter) throws Meta
       params[i + 3] = filter.params.get(i);
     }
 
-    return MetastoreDirectSqlUtils.getCountOfQuery(pm, queryText, params);
+    try (QueryWrapper query = new QueryWrapper(pm.newQuery("javax.jdo.query.SQL", queryText))) {
+      return MetastoreDirectSqlUtils.getCountOfQuery(query.getInnerQuery(), params);
+    }
   }
 
   public int getNumPartitionsViaSqlPs(Table table, List<String> partVals) throws MetaException {
@@ -1311,7 +1313,9 @@ public int getNumPartitionsViaSqlPs(Table table, List<String> partVals) throws M
     params[2] = table.getCatName();
     params[3] = partialName;
 
-    return MetastoreDirectSqlUtils.getCountOfQuery(pm, queryText, params);
+    try (QueryWrapper query = new QueryWrapper(pm.newQuery("javax.jdo.query.SQL", queryText))) {
+      return MetastoreDirectSqlUtils.getCountOfQuery(query.getInnerQuery(), params);
+    }
   }
 
   private static String trimCommaList(StringBuilder sb) {

diff --git a/...astore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDirectSqlUtils.java b/...astore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDirectSqlUtils.java
@@ -610,15 +610,9 @@ public static void throwMetaOrRuntimeException(Exception e) throws MetaException
     }
   }
 
-  static int getCountOfQuery(PersistenceManager pm, String queryText, Object[] params) {
-    boolean doTrace = LOG.isDebugEnabled();
-    long start = doTrace ? System.nanoTime() : 0;
-    try (QueryWrapper query = new QueryWrapper(pm.newQuery("javax.jdo.query.SQL", queryText))) {
-      query.setUnique(true);
-      int sqlResult = MetastoreDirectSqlUtils.extractSqlInt(query.executeWithArray(params));
-      long queryTime = doTrace ? System.nanoTime() : 0;
-      MetastoreDirectSqlUtils.timingTrace(doTrace, queryText, start, queryTime);
-      return sqlResult;
-    }
+  static int getCountOfQuery(Query query, Object[] params) {
+    query.setUnique(true);
+    int sqlResult = MetastoreDirectSqlUtils.extractSqlInt(query.executeWithArray(params));
+    return sqlResult;
   }
 }
diff --git a/...etastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/...etastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
@@ -3824,7 +3824,17 @@ protected Integer getSqlResult(GetHelper<Integer> ctx) throws MetaException {
       @Override
       protected Integer getJdoResult(GetHelper<Integer> ctx)
           throws MetaException, NoSuchObjectException, InvalidObjectException {
-        return getNumPartitionsViaOrmPs(ctx.getTable(), partVals);
+        // size is known since it contains dbName, catName, tblName and partialRegex pattern
+        Map<String, String> params = new HashMap<>(4);
+        String filter = getJDOFilterStrForPartitionVals(ctx.getTable(), partVals, params);
+        try (QueryWrapper query = new QueryWrapper(pm.newQuery(
+            "select count(partitionName) from org.apache.hadoop.hive.metastore.model.MPartition"))) {
+          query.setFilter(filter);
+          query.declareParameters(makeParameterDeclarationString(params));
+          Long result = (Long) query.executeWithMap(params);
+
+          return result.intValue();
+        }
       }
     }.run(true);
   }
@@ -3926,20 +3936,6 @@ public List<Partition> listPartitionsPsWithAuth(String catName, String db_name,
     return partitions;
   }
 
-  private Integer getNumPartitionsViaOrmPs(Table table, List<String> partVals) throws MetaException {
-    // size is known since it contains dbName, catName, tblName and partialRegex pattern
-    Map<String, String> params = new HashMap<>(4);
-    String filter = getJDOFilterStrForPartitionVals(table, partVals, params);
-    try (QueryWrapper query = new QueryWrapper(pm.newQuery(
-        "select count(partitionName) from org.apache.hadoop.hive.metastore.model.MPartition"))) {
-      query.setFilter(filter);
-      query.declareParameters(makeParameterDeclarationString(params));
-      Long result = (Long) query.executeWithMap(params);
-
-      return result.intValue();
-    }
-  }
-
   private List<Partition> getPartitionsByPs(String catName, String dbName,
                                             String tblName, GetPartitionsArgs args)
       throws MetaException, NoSuchObjectException {
@@ -3973,10 +3969,6 @@ protected List<Partition> getJdoResult(GetHelper<List<Partition>> ctx)
   @Override
   public List<String> listPartitionNamesPs(String catName, String dbName, String tableName,
       List<String> part_vals, short max_parts) throws MetaException, NoSuchObjectException {
-    if (MetaStoreUtils.arePartValsEmpty(part_vals)) {
-      return listPartitionNames(catName, dbName, tableName, max_parts);
-    }
-
     List<String> partitionNames = new ArrayList<>();
     boolean success = false;
 

diff --git a/...tore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java b/...tore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
@@ -459,13 +459,10 @@ public void testListPartitionsByValuesNullTblName() throws Exception {
     client.listPartitions(DB_NAME, null, Lists.newArrayList("1999"), (short)-1);
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testListPartitionsByValuesNullValues() throws Exception {
     createTable3PartCols1Part(client);
-    List<Partition> partitions = client.listPartitions(DB_NAME, TABLE_NAME,
-            (List<String>)null, (short)-1);
-    assertEquals(1, partitions.size());
-    assertPartitionsHaveCorrectParams(partitions);
+    client.listPartitions(DB_NAME, TABLE_NAME, (List<String>)null, (short)-1);
   }
 
 
@@ -746,7 +743,7 @@ public void testListPartitionsWithAuthByValues() throws Exception {
   public void testListPartitionsWithAuthByValuesNoVals() throws Exception {
     List<List<String>> partValues = createTable4PartColsPartsAuthOn(client).testValues;
     List<Partition> partitions = client.listPartitionsWithAuthInfo(DB_NAME, TABLE_NAME, Lists
-            .newArrayList(), (short)-1, "", Lists.newArrayList());
+        .newArrayList(), (short)-1, "", Lists.newArrayList());
     assertEquals(4, partitions.size());
     assertPartitionsHaveCorrectValues(partitions, partValues);
     assertPartitionsHaveCorrectParams(partitions);
@@ -840,14 +837,11 @@ public void testListPartitionsWithAuthByValuesNullTblName() throws Exception {
     }
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testListPartitionsWithAuthByValuesNullValues() throws Exception {
-    List<List<String>> partValues = createTable4PartColsParts(client).testValues;
-    List<Partition> partitions = client.listPartitionsWithAuthInfo(DB_NAME, TABLE_NAME,
-            (List<String>)null, (short)-1, "", Lists.newArrayList());
-    assertEquals(4, partitions.size());
-    assertPartitionsHaveCorrectValues(partitions, partValues);
-    assertPartitionsHaveCorrectParams(partitions);
+    createTable4PartColsParts(client);
+    client.listPartitionsWithAuthInfo(DB_NAME, TABLE_NAME, (List<String>)null,
+            (short)-1, "", Lists.newArrayList());
   }
 
   @Test
@@ -1426,12 +1420,10 @@ public void testListPartitionNamesByValuesLowPartCount() throws Exception {
         Lists.newArrayList("yyyy", "mm", "dd"));
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testListPartitionNamesByValuesNoPartVals() throws Exception {
     createTable4PartColsParts(client);
-    List<String> partNames = client.listPartitionNames(DB_NAME, TABLE_NAME,
-            Lists.newArrayList(), (short)-1);
-    assertEquals(4, partNames.size());
+    client.listPartitionNames(DB_NAME, TABLE_NAME, Lists.newArrayList(), (short)-1);
   }
 
   @Test(expected = MetaException.class)
@@ -1486,12 +1478,10 @@ public void testListPartitionNamesByValuesNullTblName() throws Exception {
     }
   }
 
-  @Test
+  @Test(expected = MetaException.class)
   public void testListPartitionNamesByValuesNullValues() throws Exception {
     createTable4PartColsParts(client);
-    List<String> partNames = client.listPartitionNames(DB_NAME, TABLE_NAME,
-            (List<String>)null, (short)-1);
-    assertEquals(4, partNames.size());
+    client.listPartitionNames(DB_NAME, TABLE_NAME, (List<String>)null, (short)-1);
   }