From 037cb0f09a58c36cac5664fda69593a4fa065de1 Mon Sep 17 00:00:00 2001 From: wecharyu Date: Sun, 12 May 2024 10:43:10 +0000 Subject: [PATCH 1/3] HIVE-28239: Fix bug on HMSHandler#checkLimitNumberOfPartitions --- .../hadoop/hive/metastore/HMSHandler.java | 35 +++++++------------ .../metastore/client/TestListPartitions.java | 28 +++++++++++---- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java index c77daf96fac1..0b209533bada 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java @@ -5563,26 +5563,21 @@ public List get_partitions_with_auth(final String dbName, private void checkLimitNumberOfPartitionsByFilter(String catName, String dbName, String tblName, String filterString, int maxParts) throws TException { - if (isPartitionLimitEnabled()) { + if (needCheckPartitionLimit(maxParts)) { checkLimitNumberOfPartitions(tblName, get_num_partitions_by_filter(prependCatalogToDbName( - catName, dbName, conf), tblName, filterString), maxParts); + catName, dbName, conf), tblName, filterString)); } } private void checkLimitNumberOfPartitionsByPs(String catName, String dbName, String tblName, List partVals, int maxParts) throws TException { - if (isPartitionLimitEnabled()) { + if (needCheckPartitionLimit(maxParts)) { checkLimitNumberOfPartitions(tblName, getNumPartitionsByPs(catName, dbName, tblName, - partVals), maxParts); + partVals)); } } - private boolean isPartitionLimitEnabled() { - int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); - return partitionLimit > -1; - } - // Check request partition limit iff: // 1. partition limit is enabled. // 2. request size is greater than the limit. @@ -5591,15 +5586,12 @@ private boolean needCheckPartitionLimit(int requestSize) { return partitionLimit > -1 && (requestSize < 0 || requestSize > partitionLimit); } - private void checkLimitNumberOfPartitions(String tblName, int numPartitions, int maxToFetch) throws MetaException { - if (isPartitionLimitEnabled()) { - int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); - int partitionRequest = (maxToFetch < 0) ? numPartitions : maxToFetch; - if (partitionRequest > partitionLimit) { - String configName = ConfVars.LIMIT_PARTITION_REQUEST.toString(); - throw new MetaException(String.format(PARTITION_NUMBER_EXCEED_LIMIT_MSG, partitionRequest, - tblName, partitionLimit, configName)); - } + private void checkLimitNumberOfPartitions(String tblName, int numPartitions) throws MetaException { + int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); + if (partitionLimit > -1 && numPartitions > partitionLimit) { + String configName = ConfVars.LIMIT_PARTITION_REQUEST.toString(); + throw new MetaException(String.format(PARTITION_NUMBER_EXCEED_LIMIT_MSG, numPartitions, + tblName, partitionLimit, configName)); } } @@ -7227,11 +7219,10 @@ private List get_partitions_by_filter_internal(final String catName, authorizeTableForPartitionMetadata(catName, dbName, tblName); if (needCheckPartitionLimit(args.getMax())) { // Since partition limit is configured, we need fetch at most (limit + 1) partition names - int requestMax = args.getMax(); int max = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST) + 1; args = new GetPartitionsArgs.GetPartitionsArgsBuilder(args).max(max).build(); List partNames = rs.listPartitionNamesByFilter(catName, dbName, tblName, args); - checkLimitNumberOfPartitions(tblName, partNames.size(), requestMax); + checkLimitNumberOfPartitions(tblName, partNames.size()); ret = rs.getPartitionsByNames(catName, dbName, tblName, new GetPartitionsArgs.GetPartitionsArgsBuilder(args).partNames(partNames).build()); } else { @@ -7362,7 +7353,7 @@ private PartitionsByExprResult get_partitions_by_expr_internal( // Since partition limit is configured, we need fetch at most (limit + 1) partition names int max = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST) + 1; List partNames = rs.listPartitionNames(catName, dbName, tblName, args.getDefaultPartName(), args.getExpr(), null, max); - checkLimitNumberOfPartitions(tblName, partNames.size(), args.getMax()); + checkLimitNumberOfPartitions(tblName, partNames.size()); partitions = rs.getPartitionsByNames(catName, dbName, tblName, new GetPartitionsArgs.GetPartitionsArgsBuilder(args).partNames(partNames).build()); } else { @@ -7462,7 +7453,7 @@ private List get_partitions_by_names(final String dbName, final Strin fireReadTablePreEvent(parsedCatName, parsedDbName, tblName); - checkLimitNumberOfPartitions(tblName, args.getPartNames().size(), -1); + checkLimitNumberOfPartitions(tblName, args.getPartNames().size()); ret = getMS().getPartitionsByNames(parsedCatName, parsedDbName, tblName, args); ret = FilterUtils.filterPartitionsIfEnabled(isServerFilterEnabled, filterHook, ret); table = getTable(parsedCatName, parsedDbName, tblName); diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java index 91ba1c311f33..abaf28217cae 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java @@ -203,6 +203,22 @@ private ReturnTable createTable4PartColsPartsGeneric(IMetaStoreClient client, return new ReturnTable(t, testValues); } + private ReturnTable createTableNParts(IMetaStoreClient client, boolean authOn, int nParts) + throws Exception { + Table t = createTestTable(client, DB_NAME, TABLE_NAME, Lists.newArrayList("yyyy", "mm", "dd"), + authOn); + List> testValues = new ArrayList<>(nParts); + for (int i = 0; i < nParts; i++) { + testValues.add(Lists.newArrayList("2017", "01", String.valueOf(i))); + } + + for(List vals : testValues) { + addPartition(client, t, vals); + } + + return new ReturnTable(t, testValues); + } + protected ReturnTable createTable4PartColsParts(IMetaStoreClient client) throws Exception { return createTable4PartColsPartsGeneric(client, false); @@ -324,7 +340,7 @@ public void testGetPartitionsRequest() throws Exception { @Test(expected = MetaException.class) @ConditionalIgnoreOnSessionHiveMetastoreClient public void testListPartitionsAllHighMaxParts() throws Exception { - createTable3PartCols1Part(client); + createTableNParts(client, false, 101); List partitions = client.listPartitions(DB_NAME, TABLE_NAME, (short)101); assertTrue(partitions.isEmpty()); } @@ -496,7 +512,7 @@ public void testListPartitionSpecsNoDb() throws Exception { @Test(expected = MetaException.class) @ConditionalIgnoreOnSessionHiveMetastoreClient public void testListPartitionSpecsHighMaxParts() throws Exception { - createTable4PartColsParts(client); + createTableNParts(client, false, 101); client.listPartitionSpecs(DB_NAME, TABLE_NAME, 101); } @@ -562,7 +578,7 @@ public void testListPartitionsWithAuth() throws Exception { @Test(expected = MetaException.class) @ConditionalIgnoreOnSessionHiveMetastoreClient public void testListPartitionsWithAuthHighMaxParts() throws Exception { - createTable4PartColsParts(client); + createTableNParts(client, false, 101); client.listPartitionsWithAuthInfo(DB_NAME, TABLE_NAME, (short)101, "", Lists.newArrayList()); } @@ -754,7 +770,7 @@ public void testListPartitionsWithAuthByValuesTooManyVals() throws Exception { @Test(expected = MetaException.class) @ConditionalIgnoreOnSessionHiveMetastoreClient public void testListPartitionsWithAuthByValuesHighMaxParts() throws Exception { - createTable4PartColsParts(client); + createTableNParts(client, false, 101); client.listPartitionsWithAuthInfo(DB_NAME, TABLE_NAME, Lists .newArrayList("2017"), (short) 101, "", Lists.newArrayList()); } @@ -982,7 +998,7 @@ public void testListPartitionsByFilterInvalidFilter() throws Exception { @Test(expected = MetaException.class) @ConditionalIgnoreOnSessionHiveMetastoreClient public void testListPartitionsByFilterHighMaxParts() throws Exception { - createTable4PartColsParts(client); + createTableNParts(client, false, 101); client.listPartitionsByFilter(DB_NAME, TABLE_NAME, "yyyy=\"2017\"", (short)101); } @@ -1095,7 +1111,7 @@ public void testListPartitionSpecsByFilterInvalidFilter() throws Exception { @Test(expected = MetaException.class) @ConditionalIgnoreOnSessionHiveMetastoreClient public void testListPartitionSpecsByFilterHighMaxParts() throws Exception { - createTable4PartColsParts(client); + createTableNParts(client, false, 101); client.listPartitionSpecsByFilter(DB_NAME, TABLE_NAME, "yyyy=\"2017\"", 101); } From 3d733159cadde76eec8f876e3b8083ab11ea740c Mon Sep 17 00:00:00 2001 From: wecharyu Date: Wed, 15 May 2024 06:19:27 +0000 Subject: [PATCH 2/3] reuse exceedsPartitionLimit() --- .../hadoop/hive/metastore/HMSHandler.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java index 0b209533bada..d5298fa91f0d 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java @@ -5563,7 +5563,7 @@ public List get_partitions_with_auth(final String dbName, private void checkLimitNumberOfPartitionsByFilter(String catName, String dbName, String tblName, String filterString, int maxParts) throws TException { - if (needCheckPartitionLimit(maxParts)) { + if (exceedsPartitionLimit(maxParts)) { checkLimitNumberOfPartitions(tblName, get_num_partitions_by_filter(prependCatalogToDbName( catName, dbName, conf), tblName, filterString)); } @@ -5572,23 +5572,23 @@ private void checkLimitNumberOfPartitionsByFilter(String catName, String dbName, private void checkLimitNumberOfPartitionsByPs(String catName, String dbName, String tblName, List partVals, int maxParts) throws TException { - if (needCheckPartitionLimit(maxParts)) { + if (exceedsPartitionLimit(maxParts)) { checkLimitNumberOfPartitions(tblName, getNumPartitionsByPs(catName, dbName, tblName, partVals)); } } - // Check request partition limit iff: + // Check input size exceeding partition limit iff: // 1. partition limit is enabled. - // 2. request size is greater than the limit. - private boolean needCheckPartitionLimit(int requestSize) { + // 2. input size is greater than the limit. + private boolean exceedsPartitionLimit(int size) { int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); - return partitionLimit > -1 && (requestSize < 0 || requestSize > partitionLimit); + return partitionLimit > -1 && (size < 0 || size > partitionLimit); } private void checkLimitNumberOfPartitions(String tblName, int numPartitions) throws MetaException { - int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); - if (partitionLimit > -1 && numPartitions > partitionLimit) { + if (exceedsPartitionLimit(numPartitions)) { + int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); String configName = ConfVars.LIMIT_PARTITION_REQUEST.toString(); throw new MetaException(String.format(PARTITION_NUMBER_EXCEED_LIMIT_MSG, numPartitions, tblName, partitionLimit, configName)); @@ -7217,7 +7217,7 @@ private List get_partitions_by_filter_internal(final String catName, RawStore rs = getMS(); try { authorizeTableForPartitionMetadata(catName, dbName, tblName); - if (needCheckPartitionLimit(args.getMax())) { + if (exceedsPartitionLimit(args.getMax())) { // Since partition limit is configured, we need fetch at most (limit + 1) partition names int max = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST) + 1; args = new GetPartitionsArgs.GetPartitionsArgsBuilder(args).max(max).build(); @@ -7349,7 +7349,7 @@ private PartitionsByExprResult get_partitions_by_expr_internal( List partitions = new LinkedList<>(); boolean hasUnknownPartitions = false; RawStore rs = getMS(); - if (needCheckPartitionLimit(args.getMax())) { + if (exceedsPartitionLimit(args.getMax())) { // Since partition limit is configured, we need fetch at most (limit + 1) partition names int max = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST) + 1; List partNames = rs.listPartitionNames(catName, dbName, tblName, args.getDefaultPartName(), args.getExpr(), null, max); From b7da0741fc28e14524c848f0aa1509ecdf75cef2 Mon Sep 17 00:00:00 2001 From: wecharyu Date: Mon, 20 May 2024 16:03:25 +0000 Subject: [PATCH 3/3] addressed function name --- .../hadoop/hive/metastore/HMSHandler.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java index d5298fa91f0d..486830fd0bbb 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java @@ -5562,32 +5562,32 @@ public List get_partitions_with_auth(final String dbName, private void checkLimitNumberOfPartitionsByFilter(String catName, String dbName, String tblName, String filterString, - int maxParts) throws TException { - if (exceedsPartitionLimit(maxParts)) { + int requestMax) throws TException { + if (exceedsPartitionFetchLimit(requestMax)) { checkLimitNumberOfPartitions(tblName, get_num_partitions_by_filter(prependCatalogToDbName( catName, dbName, conf), tblName, filterString)); } } private void checkLimitNumberOfPartitionsByPs(String catName, String dbName, String tblName, - List partVals, int maxParts) + List partVals, int requestMax) throws TException { - if (exceedsPartitionLimit(maxParts)) { + if (exceedsPartitionFetchLimit(requestMax)) { checkLimitNumberOfPartitions(tblName, getNumPartitionsByPs(catName, dbName, tblName, partVals)); } } - // Check input size exceeding partition limit iff: + // Check input count exceeding partition limit iff: // 1. partition limit is enabled. - // 2. input size is greater than the limit. - private boolean exceedsPartitionLimit(int size) { + // 2. input count is greater than the limit. + private boolean exceedsPartitionFetchLimit(int count) { int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); - return partitionLimit > -1 && (size < 0 || size > partitionLimit); + return partitionLimit > -1 && (count < 0 || count > partitionLimit); } private void checkLimitNumberOfPartitions(String tblName, int numPartitions) throws MetaException { - if (exceedsPartitionLimit(numPartitions)) { + if (exceedsPartitionFetchLimit(numPartitions)) { int partitionLimit = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST); String configName = ConfVars.LIMIT_PARTITION_REQUEST.toString(); throw new MetaException(String.format(PARTITION_NUMBER_EXCEED_LIMIT_MSG, numPartitions, @@ -7217,7 +7217,7 @@ private List get_partitions_by_filter_internal(final String catName, RawStore rs = getMS(); try { authorizeTableForPartitionMetadata(catName, dbName, tblName); - if (exceedsPartitionLimit(args.getMax())) { + if (exceedsPartitionFetchLimit(args.getMax())) { // Since partition limit is configured, we need fetch at most (limit + 1) partition names int max = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST) + 1; args = new GetPartitionsArgs.GetPartitionsArgsBuilder(args).max(max).build(); @@ -7349,7 +7349,7 @@ private PartitionsByExprResult get_partitions_by_expr_internal( List partitions = new LinkedList<>(); boolean hasUnknownPartitions = false; RawStore rs = getMS(); - if (exceedsPartitionLimit(args.getMax())) { + if (exceedsPartitionFetchLimit(args.getMax())) { // Since partition limit is configured, we need fetch at most (limit + 1) partition names int max = MetastoreConf.getIntVar(conf, ConfVars.LIMIT_PARTITION_REQUEST) + 1; List partNames = rs.listPartitionNames(catName, dbName, tblName, args.getDefaultPartName(), args.getExpr(), null, max);