Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HIVE-28239: Fix bug on HMSHandler#checkLimitNumberOfPartitions #5246

Merged
merged 3 commits into from
May 21, 2024
Merged

HIVE-28239: Fix bug on HMSHandler#checkLimitNumberOfPartitions #5246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
037cb0f
HIVE-28239: Fix bug on HMSHandler#checkLimitNumberOfPartitions
wecharyu May 12, 2024
3d73315
reuse exceedsPartitionLimit()
wecharyu May 15, 2024
b7da074
addressed function name
wecharyu May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 20 additions & 29 deletions ...metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5562,44 +5562,36 @@ public List<Partition> get_partitions_with_auth(final String dbName,

private void checkLimitNumberOfPartitionsByFilter(String catName, String dbName,
String tblName, String filterString,
int maxParts) throws TException {
if (isPartitionLimitEnabled()) {
int requestMax) throws TException {
if (exceedsPartitionFetchLimit(requestMax)) {
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<String> partVals, int maxParts)
List<String> partVals, int requestMax)
throws TException {
if (isPartitionLimitEnabled()) {
if (exceedsPartitionFetchLimit(requestMax)) {
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:
// Check input count exceeding partition limit iff:
// 1. partition limit is enabled.
// 2. request size is greater than the limit.
private boolean needCheckPartitionLimit(int requestSize) {
// 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 && (requestSize < 0 || requestSize > partitionLimit);
return partitionLimit > -1 && (count < 0 || count > partitionLimit);
}

private void checkLimitNumberOfPartitions(String tblName, int numPartitions, int maxToFetch) throws MetaException {
if (isPartitionLimitEnabled()) {
private void checkLimitNumberOfPartitions(String tblName, int numPartitions) throws MetaException {
if (exceedsPartitionFetchLimit(numPartitions)) {
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));
}
String configName = ConfVars.LIMIT_PARTITION_REQUEST.toString();
throw new MetaException(String.format(PARTITION_NUMBER_EXCEED_LIMIT_MSG, numPartitions,
tblName, partitionLimit, configName));
}
}

Expand Down Expand Up @@ -7225,13 +7217,12 @@ private List<Partition> get_partitions_by_filter_internal(final String catName,
RawStore rs = getMS();
try {
authorizeTableForPartitionMetadata(catName, dbName, tblName);
if (needCheckPartitionLimit(args.getMax())) {
if (exceedsPartitionFetchLimit(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<String> 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 {
Expand Down Expand Up @@ -7358,11 +7349,11 @@ private PartitionsByExprResult get_partitions_by_expr_internal(
List<Partition> partitions = new LinkedList<>();
boolean hasUnknownPartitions = false;
RawStore rs = getMS();
if (needCheckPartitionLimit(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<String> 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 {
Expand Down Expand Up @@ -7462,7 +7453,7 @@ private List<Partition> 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);
Expand Down
28 changes: 22 additions & 6 deletions ...tore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<List<String>> testValues = new ArrayList<>(nParts);
for (int i = 0; i < nParts; i++) {
testValues.add(Lists.newArrayList("2017", "01", String.valueOf(i)));
}

for(List<String> vals : testValues) {
addPartition(client, t, vals);
}

return new ReturnTable(t, testValues);
}

protected ReturnTable createTable4PartColsParts(IMetaStoreClient client) throws
Exception {
return createTable4PartColsPartsGeneric(client, false);
Expand Down Expand Up @@ -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<Partition> partitions = client.listPartitions(DB_NAME, TABLE_NAME, (short)101);
assertTrue(partitions.isEmpty());
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down