Make sure every Rule has a name

prestodb · Mar 19, 2018 · 5bcc5b8 · 5bcc5b8
1 parent d2af24f
commit 5bcc5b8
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 17 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/StatsRecorder.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/StatsRecorder.java
@@ -23,13 +23,18 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Verify.verify;
+import static java.lang.String.format;
+
 public class StatsRecorder
 {
     private final Map<Class<?>, RuleStats> stats = new HashMap<>();
 
     public void registerAll(Collection<Rule<?>> rules)
     {
         for (Rule<?> rule : rules) {
+            checkArgument(!rule.getClass().isAnonymousClass());
             stats.put(rule.getClass(), new RuleStats());
         }
     }
@@ -47,11 +52,17 @@ public void recordFailure(Rule<?> rule)
     void export(MBeanExporter exporter)
     {
         for (Map.Entry<Class<?>, RuleStats> entry : stats.entrySet()) {
+            verify(!entry.getKey().getSimpleName().isEmpty());
             String name = ObjectNames.builder(IterativeOptimizer.class)
                     .withProperty("rule", entry.getKey().getSimpleName())
                     .build();
 
-            exporter.export(name, entry.getValue());
+            try {
+                exporter.export(name, entry.getValue());
+            }
+            catch (RuntimeException e) {
+                throw new RuntimeException(format("Failed to export MBean with name '%s'", name), e);
+            }
         }
     }
 

diff --git a/...-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PreconditionRules.java b/...-main/src/main/java/com/facebook/presto/sql/planner/iterative/rule/PreconditionRules.java
@@ -33,21 +33,31 @@ public static Rule<ExchangeNode> checkRulesAreFiredBeforeAddExchangesRule()
 
     public static <T extends PlanNode> Rule<T> checkPlanDoNotMatch(Pattern<T> pattern, String message)
     {
-        requireNonNull(pattern, "pattern is null");
-        requireNonNull(message, "message is null");
-
-        return new Rule<T>() {
-            @Override
-            public Pattern<T> getPattern()
-            {
-                return pattern;
-            }
-
-            @Override
-            public Result apply(T node, Captures captures, Context context)
-            {
-                throw new IllegalStateException(message);
-            }
-        };
+        return new CheckNoPlanNodeMatchesRule<>(pattern, message);
+    }
+
+    private static class CheckNoPlanNodeMatchesRule<T extends PlanNode>
+            implements Rule<T>
+    {
+        private final Pattern<T> pattern;
+        private final String message;
+
+        public CheckNoPlanNodeMatchesRule(Pattern<T> pattern, String message)
+        {
+            this.pattern = requireNonNull(pattern, "pattern is null");
+            this.message = requireNonNull(message, "message is null");
+        }
+
+        @Override
+        public Pattern<T> getPattern()
+        {
+            return pattern;
+        }
+
+        @Override
+        public Result apply(T node, Captures captures, Context context)
+        {
+            throw new IllegalStateException(message);
+        }
     }
 }