[#11672] improvement(optimizer): optimize trigger-expr evaluation with table metadata & partition short-circuit

maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/BaseExpressionStrategyHandler.java

@@ -24,6 +24,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.PriorityQueue;
 import java.util.stream.Collectors;
 import lombok.Value;
@@ -42,6 +43,7 @@
 import org.apache.gravitino.maintenance.optimizer.recommender.util.QLExpressionEvaluator;
 import org.apache.gravitino.maintenance.optimizer.recommender.util.StatisticsUtils;
 import org.apache.gravitino.maintenance.optimizer.recommender.util.StrategyUtils;
+import org.apache.gravitino.maintenance.optimizer.recommender.util.TableMetadataTriggerExpressionUtils;
 import org.apache.gravitino.rel.Table;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -65,6 +67,8 @@ public abstract class BaseExpressionStrategyHandler implements StrategyHandler {
   private Map<PartitionPath, List<StatisticEntry<?>>> partitionStatistics;
   private Table tableMetadata;
   private NameIdentifier nameIdentifier;
+  // Cached table-level context (table stats + metadata + rules), computed once per initialize().
+  private Map<String, Object> tableLevelContext;
 
   /** Create a handler that evaluates expressions with the default QL evaluator. */
   protected BaseExpressionStrategyHandler() {
@@ -79,6 +83,7 @@ public void initialize(StrategyHandlerContext context) {
     this.strategy = context.strategy();
     this.tableStatistics = context.tableStatistics();
     this.partitionStatistics = context.partitionStatistics();
+    this.tableLevelContext = buildTableLevelContext();
   }
 
   @Override
@@ -131,16 +136,30 @@ private boolean shouldTriggerForPartitionTable() {
       return false;
     }
     String triggerExpression = triggerExpression(strategy);
-    return partitionStatistics.values().stream()
-        .anyMatch(partitionStats -> evaluateBool(triggerExpression, partitionStats));
+
+    // Short-circuit: try to evaluate the trigger expression with table-level context only
+    // (no partition statistics). This relies on the QL engine's left-to-right short-circuit
+    // evaluation of && operators: if a table-level predicate appears first and evaluates to
+    // false, the engine returns false without resolving subsequent partition-level variables.
+    // Note: this optimization only works when the table-level predicate is positioned before the
+    // partition-level predicate, e.g. "sort_order_count > 0 && datafile_mse < limit" will
+    // short-circuit, but "datafile_mse < limit && sort_order_count > 0" will not.
+    Optional<Boolean> evaluationResultWithoutPartitions =
+        tryToEvaluateBool(triggerExpression, List.of());
+    // If the trigger expression can be evaluated with table-level variables only, return the
+    // result. Otherwise, evaluate the trigger expression for each partition.
+    return evaluationResultWithoutPartitions.orElseGet(
+        () ->
+            partitionStatistics.values().stream()
+                .anyMatch(partitionStats -> evaluateBool(triggerExpression, partitionStats)));
   }
 
   private boolean shouldTriggerForNonPartitionTable() {
-    return evaluateBool(triggerExpression(strategy), tableStatistics);
+    return evaluateBool(triggerExpression(strategy), List.of());
   }
 
   private StrategyEvaluation evaluateForNonPartitionTable() {
-    long score = evaluateLong(scoreExpression(strategy), tableStatistics);
+    long score = evaluateLong(scoreExpression(strategy), List.of());
     if (score <= 0) {
       return StrategyEvaluation.NO_EXECUTION;
     }
@@ -200,7 +219,7 @@ private ScoreMode partitionTableScoreMode() {
   }
 
   private long evaluateLong(String expression, List<StatisticEntry<?>> statistics) {
-    Map<String, Object> context = buildExpressionContext(strategy, statistics);
+    Map<String, Object> context = buildExpressionContext(statistics);
     try {
       return expressionEvaluator.evaluateLong(expression, context);
     } catch (RuntimeException e) {
@@ -210,7 +229,7 @@ private long evaluateLong(String expression, List<StatisticEntry<?>> statistics)
   }
 
   private boolean evaluateBool(String expression, List<StatisticEntry<?>> statistics) {
-    Map<String, Object> context = buildExpressionContext(strategy, statistics);
+    Map<String, Object> context = buildExpressionContext(statistics);
     try {
       return expressionEvaluator.evaluateBool(expression, context);
     } catch (RuntimeException e) {
@@ -219,10 +238,47 @@ private boolean evaluateBool(String expression, List<StatisticEntry<?>> statisti
     }
   }
 
-  private static Map<String, Object> buildExpressionContext(
-      Strategy strategy, List<StatisticEntry<?>> statistics) {
-    Map<String, Object> context = new HashMap<>();
+  private Optional<Boolean> tryToEvaluateBool(
+      String expression, List<StatisticEntry<?>> statistics) {
+    Map<String, Object> context = buildExpressionContext(statistics);
+    try {
+      return expressionEvaluator.tryToEvaluateBool(expression, context);
+    } catch (RuntimeException e) {
+      LOG.warn("Failed to evaluate expression '{}' with context {}", expression, context, e);
+      // Per ExpressionEvaluator#tryToEvaluateBool, a failed evaluation yields an empty Optional so
+      // the caller falls back to per-partition evaluation instead of treating it as "do not
+      // trigger".
+      return Optional.empty();
+    }
+  }
+
+  /**
+   * Build a combined evaluation context. The {@code statistics} argument varies per partition for
+   * partitioned tables; it is layered on top of the precomputed {@link #tableLevelContext} (table
+   * statistics, table metadata, and numeric rule values) so that {@code trigger-expr} and {@code
+   * score-expr} can reference all of them.
+   *
+   * @param statistics statistics of the unit being evaluated (a single partition, or empty for the
+   *     table-level evaluation)
+   * @return combined context
+   */
+  private Map<String, Object> buildExpressionContext(List<StatisticEntry<?>> statistics) {
+    Map<String, Object> context = new HashMap<>(tableLevelContext);
     context.putAll(StatisticsUtils.buildStatisticsContext(statistics));
+    return context;
+  }
+
+  /** Precompute the table-level context shared across all partition evaluations. */
+  private Map<String, Object> buildTableLevelContext() {
+    Map<String, Object> context = new HashMap<>();
+    context.putAll(StatisticsUtils.buildStatisticsContext(tableStatistics));
+    context.putAll(TableMetadataTriggerExpressionUtils.buildTableMetadataContext(tableMetadata));
+    context.putAll(getRulesExpressionContext(strategy));
+    return context;
+  }
+
+  private static Map<String, Object> getRulesExpressionContext(Strategy strategy) {
+    Map<String, Object> context = new HashMap<>();
     strategy
         .rules()
         .forEach(

maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/ExpressionEvaluator.java

@@ -20,6 +20,7 @@
 package org.apache.gravitino.maintenance.optimizer.recommender.util;
 
 import java.util.Map;
+import java.util.Optional;
 
 /**
  * Evaluates rule expressions against a provided context map.
@@ -46,4 +47,18 @@ public interface ExpressionEvaluator {
    * @return evaluation result as a {@code long}
    */
   long evaluateLong(String expression, Map<String, Object> context);
+
+  /**
+   * Evaluates an expression that returns a boolean value and returns a {@link java.util.Optional}
+   * containing the result if the evaluation is successful, or an empty {@link java.util.Optional}
+   * if it fails.
+   *
+   * <p>Evaluation may fail if there are syntax errors in a specified expression or if a variable
+   * referenced in the expression is not present in the context.
+   *
+   * @param expression expression to evaluate
+   * @param context variable bindings for the expression
+   * @return evaluation result as an {@link java.util.Optional} containing a {@code boolean}
+   */
+  Optional<Boolean> tryToEvaluateBool(String expression, Map<String, Object> context);
 }

maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/QLExpressionEvaluator.java

@@ -22,17 +22,31 @@
 import com.alibaba.qlexpress4.Express4Runner;
 import com.alibaba.qlexpress4.InitOptions;
 import com.alibaba.qlexpress4.QLOptions;
+import com.alibaba.qlexpress4.exception.QLException;
 import com.google.common.base.Preconditions;
 import java.math.BigDecimal;
 import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class QLExpressionEvaluator implements ExpressionEvaluator {
+
+  private static final Logger LOG = LoggerFactory.getLogger(QLExpressionEvaluator.class);
+
   private static final Express4Runner RUNNER = new Express4Runner(InitOptions.DEFAULT_OPTIONS);
 
+  // Cache compiled regex patterns for hyphen-to-underscore replacement, keyed by the set of
+  // hyphenated context keys. Avoids expensive Pattern.compile on every evaluation call.
+  private final Map<Set<String>, HyphenReplacementRule> replacementRuleCache =
+      new ConcurrentHashMap<>();
+
   @Override
   public long evaluateLong(String expression, Map<String, Object> context) {
     return toLong(evaluate(expression, context));
@@ -43,6 +57,11 @@ public boolean evaluateBool(String expression, Map<String, Object> context) {
     return (boolean) evaluate(expression, context);
   }
 
+  @Override
+  public Optional<Boolean> tryToEvaluateBool(String expression, Map<String, Object> context) {
+    return tryToEvaluate(expression, context).map(o -> (Boolean) o);
+  }
+
   private Object evaluate(String expression, Map<String, Object> context) {
     Preconditions.checkArgument(StringUtils.isNotBlank(expression), "expression is blank");
     Preconditions.checkArgument(context != null, "context is null");
@@ -52,6 +71,16 @@ private Object evaluate(String expression, Map<String, Object> context) {
         .getResult();
   }
 
+  private Optional<Object> tryToEvaluate(String expression, Map<String, Object> context) {
+    try {
+      Object result = evaluate(expression, context);
+      return Optional.of(result);
+    } catch (QLException e) {
+      LOG.warn("Failed to evaluate expression '{}': {}", expression, e.getMessage());
+      return Optional.empty();
+    }
+  }
+
   private Map<String, Object> formatContextKey(Map<String, Object> context) {
     return context.entrySet().stream()
         .collect(
@@ -60,29 +89,57 @@ private Map<String, Object> formatContextKey(Map<String, Object> context) {
   }
 
   private String formatExpression(String expression, Map<String, Object> context) {
-    Map<String, String> replacements =
-        context.keySet().stream()
-            .collect(
-                Collectors.toMap(key -> key, this::normalizeIdentifier, (left, right) -> left));
-    replacements.entrySet().removeIf(entry -> entry.getKey().equals(entry.getValue()));
-    if (replacements.isEmpty()) {
+    HyphenReplacementRule rule = getReplacementRule(context);
+    if (rule == null) {
       return expression;
     }
-
-    String alternation =
-        replacements.keySet().stream().map(Pattern::quote).collect(Collectors.joining("|"));
-    Pattern pattern = Pattern.compile("(?<![A-Za-z0-9_])(" + alternation + ")(?![A-Za-z0-9_])");
-    Matcher matcher = pattern.matcher(expression);
+    Matcher matcher = rule.pattern.matcher(expression);
     StringBuffer buffer = new StringBuffer();
     while (matcher.find()) {
       String matched = matcher.group(1);
       matcher.appendReplacement(
-          buffer, Matcher.quoteReplacement(replacements.getOrDefault(matched, matched)));
+          buffer, Matcher.quoteReplacement(rule.replacements.getOrDefault(matched, matched)));
     }
     matcher.appendTail(buffer);
     return buffer.toString();
   }
 
+  /** Return a cached replacement rule for the hyphenated keys in the context, or null if none. */
+  private HyphenReplacementRule getReplacementRule(Map<String, Object> context) {
+    Set<String> hyphenatedKeys =
+        context.keySet().stream()
+            .filter(key -> !key.equals(normalizeIdentifier(key)))
+            .collect(Collectors.toSet());
+    if (hyphenatedKeys.isEmpty()) {
+      return null;
+    }
+    return replacementRuleCache.computeIfAbsent(
+        hyphenatedKeys,
+        keys -> {
+          Map<String, String> replacements =
+              keys.stream()
+                  .collect(
+                      Collectors.toMap(
+                          key -> key, this::normalizeIdentifier, (left, right) -> left));
+          String alternation =
+              replacements.keySet().stream().map(Pattern::quote).collect(Collectors.joining("|"));
+          Pattern pattern =
+              Pattern.compile("(?<![A-Za-z0-9_])(" + alternation + ")(?![A-Za-z0-9_])");
+          return new HyphenReplacementRule(pattern, replacements);
+        });
+  }
+
+  /** Pre-compiled pattern and replacement map for rewriting hyphenated identifiers. */
+  private static class HyphenReplacementRule {
+    final Pattern pattern;
+    final Map<String, String> replacements;
+
+    HyphenReplacementRule(Pattern pattern, Map<String, String> replacements) {
+      this.pattern = pattern;
+      this.replacements = replacements;
+    }
+  }
+
   private String normalizeIdentifier(String name) {
     return name.replace("-", "_");
   }

maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TableMetadataTriggerExpressionUtils.java

@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.maintenance.optimizer.recommender.util;
+
+import com.google.common.base.Preconditions;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.commons.lang3.math.NumberUtils;
+import org.apache.gravitino.rel.Table;
+
+/**
+ * Utility for translating table metadata into variables usable in a {@code trigger-expr} or {@code
+ * score-expr} expression.
+ */
+public class TableMetadataTriggerExpressionUtils {
+
+  private TableMetadataTriggerExpressionUtils() {}
+
+  /**
+   * Builds a context map for evaluating table metadata trigger expressions.
+   *
+   * <p>This method creates a map containing:
+   *
+   * <ul>
+   *   <li>Built-in table metadata: {@code column_count}, {@code partition_count}, {@code
+   *       sort_order_count}
+   *   <li>All table properties from {@link Table#properties()}, with numeric strings converted to
+   *       {@code long} values and all others kept as {@code String} values
+   * </ul>
+   *
+   * @param tableMetadata the table metadata to extract properties from
+   * @return a context map suitable for expression evaluation
+   */
+  public static Map<String, Object> buildTableMetadataContext(Table tableMetadata) {
+    Preconditions.checkArgument(tableMetadata != null, "Table metadata is null");
+    Map<String, Object> context = new HashMap<>();
+    context.put(
+        TableMetadataTriggerExpression.COLUMN_COUNT.getName(),
+        tableMetadata.columns() != null ? tableMetadata.columns().length : 0);
+    context.put(
+        TableMetadataTriggerExpression.PARTITION_COUNT.getName(),
+        tableMetadata.partitioning() != null ? tableMetadata.partitioning().length : 0);
+    context.put(
+        TableMetadataTriggerExpression.SORT_ORDER_COUNT.getName(),
+        tableMetadata.sortOrder() != null ? tableMetadata.sortOrder().length : 0);
+
+    if (tableMetadata.properties() != null) {
+      tableMetadata
+          .properties()
+          .forEach(
+              (k, v) -> {
+                if (NumberUtils.isCreatable(v)) {
+                  context.put(k, NumberUtils.createNumber(v).longValue());
+                } else {
+                  // For non-numeric properties, we just put the value as is.
+                  context.put(k, v);
+                }
+              });
+    }
+    return context;
+  }
+
+  /** Supported built-in table metadata trigger expression variables. */
+  private enum TableMetadataTriggerExpression {
+    COLUMN_COUNT,
+    PARTITION_COUNT,
+    SORT_ORDER_COUNT;
+
+    public String getName() {
+      return name().toLowerCase();
+    }
+  }
+}