deniskuzZ commented on code in PR #5466:
URL: https://github.com/apache/hive/pull/5466#discussion_r1778421797
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -7247,7 +7247,7 @@ public List<Map.Entry<String, String>>
getMatchingEntries(Pattern regex) {
for (Map.Entry<String, String> entry : this) {
Review Comment:
that is some crazy code, could you please refactor it with below snippet and
extract to HS2 (where it's used)
````
public Map<String, Integer> getPoolConfigEntries(HiveConf hiveConf) {
Map<String, Integer> poolConfigs = new HashMap<>();
for (Map.Entry<String, String> entry : hiveConf) {
Matcher matcher = COMPACTION_POOLS_PATTERN.matcher(entry.getKey());
if (matcher.matches()) {
matchingEntries.put(matcher.group(1),
NumberUtils.toInt(entry.getValue(), 0));
}
}
return poolConfigs;
}
````
wonder how it was even tested?
##########
common/src/test/org/apache/hadoop/hive/conf/TestHiveConf.java:
##########
@@ -252,4 +254,16 @@ public void testAdditionalConfigFiles() throws Exception{
f2.delete();
fileBakHiveSite.delete();
}
+
+ @Test
+ public void testGetMatchingEntries() {
+ HiveConf conf = new HiveConf();
+ conf.setInt("hive.compactor.worker.iceberg.threads", 4);
+
+ List<Map.Entry<String, String>> entries =
conf.getMatchingEntries(Constants.COMPACTION_POOLS_PATTERN);
Review Comment:
why is that a `List<Map.Entry<String, String>>` and not just Map<String,
String>?
--
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]
