Apply some coderabbitai comments

Author: anlowee

presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpMetadataFilterProvider.java

@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Function;
 
 import static com.facebook.presto.plugin.clp.ClpConnectorFactory.CONNECTOR_NAME;
 import static com.facebook.presto.plugin.clp.ClpErrorCode.CLP_MANDATORY_METADATA_FILTER_NOT_VALID;
@@ -97,7 +98,7 @@ public void checkContainsRequiredFilters(SchemaTableName schemaTableName, String
     {
         boolean hasRequiredMetadataFilterColumns = true;
         ImmutableList.Builder<String> notFoundListBuilder = ImmutableList.builder();
-        for (String columnName : getRequiredFilterNames(format("%s.%s", CONNECTOR_NAME, schemaTableName))) {
+        for (String columnName : getRequiredColumnNames(format("%s.%s", CONNECTOR_NAME, schemaTableName))) {
             if (!metadataFilterSql.contains(columnName)) {
                 hasRequiredMetadataFilterColumns = false;
                 notFoundListBuilder.add(columnName);
@@ -166,52 +167,42 @@ public String remapFilterSql(String scope, String sql)
         return remappedSql;
     }
 
-    public Set<String> getFilterNames(String scope)
+    public Set<String> getColumnNames(String scope)
     {
-        String[] splitScope = scope.split("\\.");
-        if (0 == splitScope.length) {
-            return ImmutableSet.of();
-        }
-        ImmutableSet.Builder<String> builder = ImmutableSet.builder();
-        builder.addAll(getAllFilterNamesFromFilters(filterMap.get(splitScope[0])));
-
-        if (1 < splitScope.length) {
-            builder.addAll(getAllFilterNamesFromFilters(filterMap.get(splitScope[0] + "." + splitScope[1])));
-        }
-
-        if (3 == splitScope.length) {
-            builder.addAll(getAllFilterNamesFromFilters(filterMap.get(scope)));
-        }
-        return builder.build();
+        return collectColumnNamesFromScopes(scope, this::getAllColumnNamesFromFilters);
     }
 
-    private Set<String> getAllFilterNamesFromFilters(List<Filter> filters)
+    private Set<String> getRequiredColumnNames(String scope)
     {
-        return null != filters ? filters.stream()
-                .map(filter -> filter.columnName)
-                .collect(toImmutableSet()) : ImmutableSet.of();
+        return collectColumnNamesFromScopes(scope, this::getRequiredColumnNamesFromFilters);
     }
 
-    private Set<String> getRequiredFilterNames(String scope)
+    private Set<String> collectColumnNamesFromScopes(String scope, Function<List<Filter>, Set<String>> extractor)
     {
         String[] splitScope = scope.split("\\.");
-        if (0 == splitScope.length) {
-            return ImmutableSet.of();
-        }
         ImmutableSet.Builder<String> builder = ImmutableSet.builder();
-        builder.addAll(getRequiredFilterNamesFromFilters(filterMap.get(splitScope[0])));
 
-        if (1 < splitScope.length) {
-            builder.addAll(getRequiredFilterNamesFromFilters(filterMap.get(splitScope[0] + "." + splitScope[1])));
+        builder.addAll(extractor.apply(filterMap.get(splitScope[0])));
+
+        if (splitScope.length > 1) {
+            builder.addAll(extractor.apply(filterMap.get(splitScope[0] + "." + splitScope[1])));
         }
 
-        if (3 == splitScope.length) {
-            builder.addAll(getRequiredFilterNamesFromFilters(filterMap.get(scope)));
+        if (splitScope.length == 3) {
+            builder.addAll(extractor.apply(filterMap.get(scope)));
         }
+
         return builder.build();
     }
 
-    private Set<String> getRequiredFilterNamesFromFilters(List<Filter> filters)
+    private Set<String> getAllColumnNamesFromFilters(List<Filter> filters)
+    {
+        return null != filters ? filters.stream()
+                .map(filter -> filter.columnName)
+                .collect(toImmutableSet()) : ImmutableSet.of();
+    }
+
+    private Set<String> getRequiredColumnNamesFromFilters(List<Filter> filters)
     {
         return null != filters ? filters.stream()
                 .filter(filter -> filter.required)

presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java

@@ -84,7 +84,7 @@ public PlanNode visitFilter(FilterNode node, RewriteContext<Void> context)
                             functionResolution,
                             functionManager,
                             assignments,
-                            metadataFilterProvider.getFilterNames(scope)), null);
+                            metadataFilterProvider.getColumnNames(scope)), null);
             Optional<String> kqlQuery = clpExpression.getPushDownExpression();
             Optional<String> metadataSqlQuery = clpExpression.getMetadataSqlQuery();
             Optional<RowExpression> remainingPredicate = clpExpression.getRemainingExpression();

presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpMetadataFilterConfig.java

@@ -66,11 +66,11 @@ public void getFilterNames()
         ClpConfig config = new ClpConfig();
         config.setMetadataFilterConfig(filterConfigPath);
         ClpMetadataFilterProvider filterProvider = new ClpMetadataFilterProvider(config);
-        Set<String> catalogFilterNames = filterProvider.getFilterNames("clp");
+        Set<String> catalogFilterNames = filterProvider.getColumnNames("clp");
         assertEquals(ImmutableSet.of("level"), catalogFilterNames);
-        Set<String> schemaFilterNames = filterProvider.getFilterNames("clp.default");
+        Set<String> schemaFilterNames = filterProvider.getColumnNames("clp.default");
         assertEquals(ImmutableSet.of("level", "author"), schemaFilterNames);
-        Set<String> tableFilterNames = filterProvider.getFilterNames("clp.default.table_1");
+        Set<String> tableFilterNames = filterProvider.getColumnNames("clp.default.table_1");
         assertEquals(ImmutableSet.of("level", "author", "msg.timestamp", "file_name"), tableFilterNames);
     }