lasdf1234 commented on code in PR #14675:
URL: https://github.com/apache/gravitino/pull/14675#discussion_r4490552454
##########
maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/recommender/handler/BaseExpressionStrategyHandler.java:
##########
@@ -241,40 +235,29 @@ private boolean evaluateBool(String expression,
List<StatisticEntry<?>> statisti
}
}
- 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.
+ * Builds the expression context for one evaluation unit.
+ *
+ * <p>Partition-aware handlers on partitioned tables receive only {@link
#metadataAndRulesContext}
+ * plus the partition's statistics. Non-partitioned evaluation also includes
table-level
+ * statistics.
*
- * @param statistics statistics of the unit being evaluated (a single
partition, or empty for the
- * table-level evaluation)
+ * @param statistics statistics of the unit being evaluated (a single
partition, or empty for
+ * non-partitioned table-level evaluation)
* @return combined context
*/
private Map<String, Object> buildExpressionContext(List<StatisticEntry<?>>
statistics) {
- Map<String, Object> context = new HashMap<>(tableLevelContext);
+ Map<String, Object> context = new HashMap<>(metadataAndRulesContext);
+ if (!isPartitionTable()) {
+ context.putAll(StatisticsUtils.buildStatisticsContext(tableStatistics));
Review Comment:
Good catch — fixed in 1084f47b9a.
Non-partition evaluation now applies table statistics before
`metadataAndRulesContext` again (stats → metadata → rules), while partitioned
evaluation still excludes table-level stats. Added
`testNonPartitionTablePrefersMetadataOverDuplicateStatisticKey` to lock the
precedence.
--
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]
