diff --git a/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/BaseExpressionStrategyHandler.java b/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/BaseExpressionStrategyHandler.java index 5c0aa596631..07072adfda0 100644 --- a/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/BaseExpressionStrategyHandler.java +++ b/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>> partitionStatistics; private Table tableMetadata; private NameIdentifier nameIdentifier; + // Cached table-level context (table stats + metadata + rules), computed once per initialize(). + private Map 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 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> statistics) { - Map context = buildExpressionContext(strategy, statistics); + Map context = buildExpressionContext(statistics); try { return expressionEvaluator.evaluateLong(expression, context); } catch (RuntimeException e) { @@ -210,7 +229,7 @@ private long evaluateLong(String expression, List> statistics) } private boolean evaluateBool(String expression, List> statistics) { - Map context = buildExpressionContext(strategy, statistics); + Map context = buildExpressionContext(statistics); try { return expressionEvaluator.evaluateBool(expression, context); } catch (RuntimeException e) { @@ -219,10 +238,47 @@ private boolean evaluateBool(String expression, List> statisti } } - private static Map buildExpressionContext( - Strategy strategy, List> statistics) { - Map context = new HashMap<>(); + private Optional tryToEvaluateBool( + String expression, List> statistics) { + Map 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 buildExpressionContext(List> statistics) { + Map context = new HashMap<>(tableLevelContext); context.putAll(StatisticsUtils.buildStatisticsContext(statistics)); + return context; + } + + /** Precompute the table-level context shared across all partition evaluations. */ + private Map buildTableLevelContext() { + Map context = new HashMap<>(); + context.putAll(StatisticsUtils.buildStatisticsContext(tableStatistics)); + context.putAll(TableMetadataTriggerExpressionUtils.buildTableMetadataContext(tableMetadata)); + context.putAll(getRulesExpressionContext(strategy)); + return context; + } + + private static Map getRulesExpressionContext(Strategy strategy) { + Map context = new HashMap<>(); strategy .rules() .forEach( diff --git a/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/ExpressionEvaluator.java b/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/ExpressionEvaluator.java index 71d8a4d6e15..fb6462b8e34 100644 --- a/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/ExpressionEvaluator.java +++ b/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 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. + * + *

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 tryToEvaluateBool(String expression, Map context); } diff --git a/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/QLExpressionEvaluator.java b/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/QLExpressionEvaluator.java index cd7609ebca7..1a547aeac62 100644 --- a/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/QLExpressionEvaluator.java +++ b/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, HyphenReplacementRule> replacementRuleCache = + new ConcurrentHashMap<>(); + @Override public long evaluateLong(String expression, Map context) { return toLong(evaluate(expression, context)); @@ -43,6 +57,11 @@ public boolean evaluateBool(String expression, Map context) { return (boolean) evaluate(expression, context); } + @Override + public Optional tryToEvaluateBool(String expression, Map context) { + return tryToEvaluate(expression, context).map(o -> (Boolean) o); + } + private Object evaluate(String expression, Map 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 context) { .getResult(); } + private Optional tryToEvaluate(String expression, Map 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 formatContextKey(Map context) { return context.entrySet().stream() .collect( @@ -60,29 +89,57 @@ private Map formatContextKey(Map context) { } private String formatExpression(String expression, Map context) { - Map 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("(? context) { + Set hyphenatedKeys = + context.keySet().stream() + .filter(key -> !key.equals(normalizeIdentifier(key))) + .collect(Collectors.toSet()); + if (hyphenatedKeys.isEmpty()) { + return null; + } + return replacementRuleCache.computeIfAbsent( + hyphenatedKeys, + keys -> { + Map 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("(? replacements; + + HyphenReplacementRule(Pattern pattern, Map replacements) { + this.pattern = pattern; + this.replacements = replacements; + } + } + private String normalizeIdentifier(String name) { return name.replace("-", "_"); } diff --git a/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TableMetadataTriggerExpressionUtils.java b/maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TableMetadataTriggerExpressionUtils.java new file mode 100644 index 00000000000..24e539518e4 --- /dev/null +++ b/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. + * + *

This method creates a map containing: + * + *

    + *
  • Built-in table metadata: {@code column_count}, {@code partition_count}, {@code + * sort_order_count} + *
  • All table properties from {@link Table#properties()}, with numeric strings converted to + * {@code long} values and all others kept as {@code String} values + *
+ * + * @param tableMetadata the table metadata to extract properties from + * @return a context map suitable for expression evaluation + */ + public static Map buildTableMetadataContext(Table tableMetadata) { + Preconditions.checkArgument(tableMetadata != null, "Table metadata is null"); + Map 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(); + } + } +} diff --git a/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/compaction/TestCompactionStrategyHandler.java b/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/compaction/TestCompactionStrategyHandler.java index 231204b6964..300949a6799 100644 --- a/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/compaction/TestCompactionStrategyHandler.java +++ b/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/compaction/TestCompactionStrategyHandler.java @@ -39,6 +39,9 @@ import org.apache.gravitino.maintenance.optimizer.recommender.util.StrategyUtils; import org.apache.gravitino.rel.Column; import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.expressions.NamedReference; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; +import org.apache.gravitino.rel.expressions.sorts.SortOrders; import org.apache.gravitino.rel.expressions.transforms.Transforms; import org.apache.gravitino.stats.StatisticValues; import org.junit.jupiter.api.Assertions; @@ -124,6 +127,228 @@ void testShouldTriggerActionWithPartitions() { Assertions.assertFalse(lowHandler.shouldTrigger()); } + @Test + void testShouldTriggerUsingTableMetadata() { + NameIdentifier tableId = NameIdentifier.of("db", "table"); + Map rules = new HashMap<>(); + rules.put(StrategyUtils.TRIGGER_EXPR, "sort_order_count > 0 && min_target_size > 100"); + rules.put(StrategyUtils.SCORE_EXPR, "1"); + Strategy metadataStrategy = + new Strategy() { + @Override + public String name() { + return "metadata-trigger-test"; + } + + @Override + public String strategyType() { + return CompactionStrategyHandler.NAME; + } + + @Override + public Map rules() { + return rules; + } + + @Override + public Map properties() { + return Map.of(); + } + + @Override + public Map jobOptions() { + return Map.of(); + } + + @Override + public String jobTemplateName() { + return "compaction-template"; + } + }; + + // A sorted table whose property satisfies the threshold should trigger. + Table sorted = Mockito.mock(Table.class); + Mockito.when(sorted.partitioning()) + .thenReturn(new org.apache.gravitino.rel.expressions.transforms.Transform[0]); + Mockito.when(sorted.sortOrder()) + .thenReturn(new org.apache.gravitino.rel.expressions.sorts.SortOrder[1]); + Mockito.when(sorted.properties()).thenReturn(Map.of("min_target_size", "500")); + StrategyHandlerContext sortedContext = + StrategyHandlerContext.builder(tableId, metadataStrategy) + .withTableMetadata(sorted) + .withTableStatistics(List.of()) + .build(); + CompactionStrategyHandler sortedHandler = new CompactionStrategyHandler(); + sortedHandler.initialize(sortedContext); + Assertions.assertTrue(sortedHandler.shouldTrigger()); + + // An unsorted table (sort_order_count == 0) must not trigger the same expression. + Table unsorted = Mockito.mock(Table.class); + Mockito.when(unsorted.partitioning()) + .thenReturn(new org.apache.gravitino.rel.expressions.transforms.Transform[0]); + Mockito.when(unsorted.sortOrder()) + .thenReturn(new org.apache.gravitino.rel.expressions.sorts.SortOrder[0]); + Mockito.when(unsorted.properties()).thenReturn(Map.of("min_target_size", "500")); + StrategyHandlerContext unsortedContext = + StrategyHandlerContext.builder(tableId, metadataStrategy) + .withTableMetadata(unsorted) + .withTableStatistics(List.of()) + .build(); + CompactionStrategyHandler unsortedHandler = new CompactionStrategyHandler(); + unsortedHandler.initialize(unsortedContext); + Assertions.assertFalse(unsortedHandler.shouldTrigger()); + } + + @Test + void testShouldTriggerShortCircuitsWhenTableLevelExprEvaluatesToFalse() { + NameIdentifier tableId = NameIdentifier.of("db", "table"); + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.partitioning()) + .thenReturn( + new org.apache.gravitino.rel.expressions.transforms.Transform[] { + Transforms.identity("table") + }); + Mockito.when(tableMetadata.sortOrder()).thenReturn(new SortOrder[0]); + Mockito.when(tableMetadata.columns()).thenReturn(new Column[0]); + Mockito.when(tableMetadata.properties()).thenReturn(Map.of()); + + // Since sort_order_count == 0, QL evaluator short-circuits the && and returns false + // without needing to resolve the partition-level variable + Strategy strategy = buildStrategyWithTriggerExpr("sort_order_count > 0 && datafile_mse > 0"); + + Map>> partitionStats = + Map.of( + PartitionPath.of(Arrays.asList(new PartitionEntryImpl("table", "table_1"))), + List.of(new StatisticEntryImpl("datafile_mse", StatisticValues.longValue(10L))), + PartitionPath.of(Arrays.asList(new PartitionEntryImpl("table", "table_2"))), + List.of(new StatisticEntryImpl("datafile_mse", StatisticValues.longValue(20L)))); + + StrategyHandlerContext context = + StrategyHandlerContext.builder(tableId, strategy) + .withTableMetadata(tableMetadata) + .withTableStatistics(List.of()) + .withPartitionStatistics(partitionStats) + .build(); + + CompactionStrategyHandler handler = new CompactionStrategyHandler(); + handler.initialize(context); + Assertions.assertFalse( + handler.shouldTrigger(), + "Should short-circuit to false when table-level predicate is first and evaluates to false"); + } + + @Test + void testShouldTriggerShortCircuitsWhenTableLevelExprEvaluatesToTrue() { + NameIdentifier tableId = NameIdentifier.of("db", "table"); + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.partitioning()) + .thenReturn( + new org.apache.gravitino.rel.expressions.transforms.Transform[] { + Transforms.identity("table") + }); + Mockito.when(tableMetadata.sortOrder()) + .thenReturn(new SortOrder[] {SortOrders.ascending(NamedReference.field("db"))}); + Mockito.when(tableMetadata.columns()).thenReturn(new Column[0]); + Mockito.when(tableMetadata.properties()).thenReturn(Map.of()); + + // Since sort_order_count == 1, QL evaluator short-circuits the || and returns true + // without needing to resolve the partition-level variable + Strategy strategy = buildStrategyWithTriggerExpr("sort_order_count > 0 || datafile_mse == 0"); + + Map>> partitionStats = + Map.of( + PartitionPath.of(Arrays.asList(new PartitionEntryImpl("table", "table_1"))), + List.of(new StatisticEntryImpl("datafile_mse", StatisticValues.longValue(10L))), + PartitionPath.of(Arrays.asList(new PartitionEntryImpl("table", "table_2"))), + List.of(new StatisticEntryImpl("datafile_mse", StatisticValues.longValue(20L)))); + + StrategyHandlerContext context = + StrategyHandlerContext.builder(tableId, strategy) + .withTableMetadata(tableMetadata) + .withTableStatistics(List.of()) + .withPartitionStatistics(partitionStats) + .build(); + + CompactionStrategyHandler handler = new CompactionStrategyHandler(); + handler.initialize(context); + Assertions.assertTrue( + handler.shouldTrigger(), + "Should short-circuit to true when table-level predicate is first and evaluates to true"); + } + + @Test + void testShouldTriggerShortCircuitsWhenTableLevelOnlyExpr() { + NameIdentifier tableId = NameIdentifier.of("db", "table"); + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.partitioning()) + .thenReturn( + new org.apache.gravitino.rel.expressions.transforms.Transform[] { + Transforms.identity("p") + }); + Mockito.when(tableMetadata.sortOrder()) + .thenReturn(new SortOrder[] {SortOrders.ascending(NamedReference.field("id"))}); + Mockito.when(tableMetadata.columns()).thenReturn(new Column[0]); + Mockito.when(tableMetadata.properties()).thenReturn(Map.of()); + + // Expression uses only table-level variable, should evaluate once without partition iteration + Strategy strategy = buildStrategyWithTriggerExpr("sort_order_count > 0"); + + Map>> partitionStats = + Map.of( + PartitionPath.of(Arrays.asList(new PartitionEntryImpl("p", "1"))), + List.of(new StatisticEntryImpl("datafile_mse", StatisticValues.longValue(10L)))); + + StrategyHandlerContext context = + StrategyHandlerContext.builder(tableId, strategy) + .withTableMetadata(tableMetadata) + .withTableStatistics(List.of()) + .withPartitionStatistics(partitionStats) + .build(); + + CompactionStrategyHandler handler = new CompactionStrategyHandler(); + handler.initialize(context); + Assertions.assertTrue( + handler.shouldTrigger(), + "Should short-circuit to true when table-level-only expression evaluates to true"); + } + + @Test + void testShouldTriggerFallsBackToPartitionEvalWhenTableLevelExprIsNotFirst() { + NameIdentifier tableId = NameIdentifier.of("db", "table"); + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.partitioning()) + .thenReturn( + new org.apache.gravitino.rel.expressions.transforms.Transform[] { + Transforms.identity("table") + }); + Mockito.when(tableMetadata.sortOrder()).thenReturn(new SortOrder[0]); + Mockito.when(tableMetadata.columns()).thenReturn(new Column[0]); + Mockito.when(tableMetadata.properties()).thenReturn(Map.of()); + + // Partition-level predicate first: datafile_mse > 0 && sort_order_count > 0 + // tryToEvaluateBool will fail on missing 'datafile_mse', so falls back to per-partition eval + Strategy strategy = buildStrategyWithTriggerExpr("datafile_mse > 0 && sort_order_count > 0"); + + Map>> partitionStats = + Map.of( + PartitionPath.of(Arrays.asList(new PartitionEntryImpl("table", "table_1"))), + List.of(new StatisticEntryImpl("datafile_mse", StatisticValues.longValue(10L)))); + + StrategyHandlerContext context = + StrategyHandlerContext.builder(tableId, strategy) + .withTableMetadata(tableMetadata) + .withTableStatistics(List.of()) + .withPartitionStatistics(partitionStats) + .build(); + + CompactionStrategyHandler handler = new CompactionStrategyHandler(); + handler.initialize(context); + // sort_order_count == 0 so per-partition eval will also return false + Assertions.assertFalse( + handler.shouldTrigger(), + "Falls back to per-partition eval; still false because sort_order_count is 0"); + } + @Test void testJobConfig() { NameIdentifier tableId = NameIdentifier.of("db", "table"); @@ -417,6 +642,45 @@ private long evaluatePartitionScore( return evaluation.score(); } + private Strategy buildStrategyWithTriggerExpr(String triggerExpr) { + Map rules = new HashMap<>(); + if (triggerExpr != null) { + rules.put(StrategyUtils.TRIGGER_EXPR, triggerExpr); + } + rules.put(StrategyUtils.SCORE_EXPR, "datafile_mse"); + return new Strategy() { + @Override + public String name() { + return "compaction-test"; + } + + @Override + public String strategyType() { + return CompactionStrategyHandler.NAME; + } + + @Override + public Map rules() { + return rules; + } + + @Override + public Map properties() { + return Map.of(); + } + + @Override + public Map jobOptions() { + return Map.of(); + } + + @Override + public String jobTemplateName() { + return "compaction-template"; + } + }; + } + private Strategy buildStrategy(ScoreMode scoreMode, Integer maxPartitionNum) { Map rules = new HashMap<>(); rules.put(StrategyUtils.TRIGGER_EXPR, "datafile_mse > 0"); diff --git a/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TestQLExpressionEvaluator.java b/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TestQLExpressionEvaluator.java index cf372fcd383..7d7877ab98a 100644 --- a/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TestQLExpressionEvaluator.java +++ b/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TestQLExpressionEvaluator.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -167,4 +168,68 @@ void testHyphenatedIdentifierNextToDotIsNotRewritten() { long result = evaluator.evaluateLong("a + metric-1", context); assertEquals(5L, result); } + + @Test + void testTryToEvaluateBoolWithResult() { + Map context = new HashMap<>(); + context.put("x", 5); + context.put("y", 10); + + Optional result = evaluator.tryToEvaluateBool("x < y", context); + assertEquals(Optional.of(true), result); + } + + @Test + void testTryToEvaluateBoolWithMissingVariableReturnsEmpty() { + Map context = new HashMap<>(); + context.put("x", 5); + + Optional result = evaluator.tryToEvaluateBool("x < y", context); + assertEquals(Optional.empty(), result); + } + + @Test + void testTryToEvaluateBoolWithBlankExpression() { + assertThrows(IllegalArgumentException.class, () -> evaluator.tryToEvaluateBool("", Map.of())); + assertThrows(IllegalArgumentException.class, () -> evaluator.tryToEvaluateBool(null, Map.of())); + } + + @Test + void testRepeatedEvaluationsWithSameHyphenatedKeysProduceCorrectResults() { + Map context1 = new HashMap<>(); + context1.put("metric-a", 10); + context1.put("threshold", 5); + assertTrue(evaluator.evaluateBool("metric-a > threshold", context1)); + + // Same hyphenated key set, different values — should reuse cached replacement rule + Map context2 = new HashMap<>(); + context2.put("metric-a", 3); + context2.put("threshold", 5); + Assertions.assertFalse(evaluator.evaluateBool("metric-a > threshold", context2)); + } + + @Test + void testDifferentHyphenatedKeySetsProduceCorrectResults() { + Map contextA = new HashMap<>(); + contextA.put("metric-a", 10); + contextA.put("limit", 5); + assertEquals(5L, evaluator.evaluateLong("metric-a - limit", contextA)); + + // Different hyphenated key set — must not reuse the previous cached rule + Map contextB = new HashMap<>(); + contextB.put("metric-b", 20); + contextB.put("limit", 8); + assertEquals(12L, evaluator.evaluateLong("metric-b - limit", contextB)); + } + + @Test + void testContextWithoutHyphenatedKeysBypassesReplacementCache() { + Map context = new HashMap<>(); + context.put("alpha", 7); + context.put("beta", 3); + + // No hyphenated keys — cache should not be involved, expression evaluated as-is + assertEquals(10L, evaluator.evaluateLong("alpha + beta", context)); + assertTrue(evaluator.evaluateBool("alpha > beta", context)); + } } diff --git a/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TestTableMetadataTriggerExpressionUtils.java b/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TestTableMetadataTriggerExpressionUtils.java new file mode 100644 index 00000000000..0e0c94af2ff --- /dev/null +++ b/maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/recommender/util/TestTableMetadataTriggerExpressionUtils.java @@ -0,0 +1,118 @@ +/* + * 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 java.util.Map; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; +import org.apache.gravitino.rel.expressions.transforms.Transform; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +class TestTableMetadataTriggerExpressionUtils { + + @Test + void testBuildTableMetadataContextWithAllFields() { + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.columns()).thenReturn(new Column[3]); + Mockito.when(tableMetadata.partitioning()).thenReturn(new Transform[1]); + Mockito.when(tableMetadata.sortOrder()).thenReturn(new SortOrder[1]); + Mockito.when(tableMetadata.properties()) + .thenReturn(Map.of("format", "iceberg", "max_file_size", "1073741824")); + + Map context = + TableMetadataTriggerExpressionUtils.buildTableMetadataContext(tableMetadata); + + Assertions.assertEquals(5, context.size()); + Assertions.assertEquals(3, context.get("column_count")); + Assertions.assertEquals(1, context.get("partition_count")); + Assertions.assertEquals(1, context.get("sort_order_count")); + Assertions.assertEquals("iceberg", context.get("format")); + Assertions.assertEquals(1073741824L, context.get("max_file_size")); + } + + @Test + void testBuildTableMetadataContextWithEmptyTableMetadata() { + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.columns()).thenReturn(null); + Mockito.when(tableMetadata.partitioning()).thenReturn(null); + Mockito.when(tableMetadata.sortOrder()).thenReturn(null); + Mockito.when(tableMetadata.properties()).thenReturn(Map.of()); + + Map context = + TableMetadataTriggerExpressionUtils.buildTableMetadataContext(tableMetadata); + + Assertions.assertEquals(3, context.size()); + Assertions.assertEquals(0, context.get("column_count")); + Assertions.assertEquals(0, context.get("partition_count")); + Assertions.assertEquals(0, context.get("sort_order_count")); + } + + @Test + void testBuildTableMetadataContextWithNullProperties() { + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.columns()).thenReturn(new Column[1]); + Mockito.when(tableMetadata.partitioning()).thenReturn(new Transform[0]); + Mockito.when(tableMetadata.sortOrder()).thenReturn(new SortOrder[0]); + Mockito.when(tableMetadata.properties()).thenReturn(null); + + Map context = + TableMetadataTriggerExpressionUtils.buildTableMetadataContext(tableMetadata); + + Assertions.assertEquals(3, context.size()); + Assertions.assertEquals(1, context.get("column_count")); + Assertions.assertEquals(0, context.get("partition_count")); + Assertions.assertEquals(0, context.get("sort_order_count")); + } + + @Test + void testBuildTableMetadataContextParsesNumericProperties() { + Table tableMetadata = Mockito.mock(Table.class); + Mockito.when(tableMetadata.columns()).thenReturn(new Column[1]); + Mockito.when(tableMetadata.partitioning()).thenReturn(new Transform[0]); + Mockito.when(tableMetadata.sortOrder()).thenReturn(new SortOrder[0]); + Mockito.when(tableMetadata.properties()) + .thenReturn( + Map.of( + "count", "12345", + "size", "9876543210", + "name", "test_table", + "enabled", "true")); + + Map context = + TableMetadataTriggerExpressionUtils.buildTableMetadataContext(tableMetadata); + + Assertions.assertEquals(12345L, context.get("count")); + Assertions.assertEquals(9876543210L, context.get("size")); + Assertions.assertEquals("test_table", context.get("name")); + Assertions.assertEquals("true", context.get("enabled")); + } + + @Test + void testBuildTableMetadataContextFailsWhenTableMetadataIsNull() { + IllegalArgumentException exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> TableMetadataTriggerExpressionUtils.buildTableMetadataContext(null)); + Assertions.assertTrue(exception.getMessage().contains("Table metadata is null")); + } +}