github-actions[bot] commented on code in PR #69760:
URL: https://github.com/apache/doris/pull/69760#discussion_r4421783850
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateNotNull.java:
##########
@@ -77,24 +88,42 @@ public List<Rule> buildRules() {
);
}
- private List<Expression> removeGeneratedNotNull(Collection<Expression>
exprs, CascadesContext ctx) {
+ /**
+ * Remove the generated `is not null` predicates, but keep the ones worth
pruning the scan.
+ *
+ * @param filterChild the child plan of the filter which owns the
predicates, the generated
+ * `is not null` predicate can only be kept when the
predicates are evaluated
+ * directly on a scan; empty if the predicates are join
conditions.
+ */
+ private List<Expression> removeGeneratedNotNull(Collection<Expression>
exprs, CascadesContext ctx,
+ Optional<Plan> filterChild) {
// Example: `id > 0 and id is not null and name is not null(generated)`
// predicatesNotContainIsNotNull: `id > 0`
// predicatesNotContainIsNotNull infer nonNullable slots: `id`
// slotsFromIsNotNull: `id`, `name`
// remove `name` (it's generated), remove `id` (because `id > 0`
already contains it)
Set<Expression> predicatesNotContainIsNotNull =
Sets.newLinkedHashSet();
List<Slot> slotsFromIsNotNull = Lists.newArrayList();
+ OlapScan scan = filterChild.filter(OlapScan.class::isInstance)
+ .map(OlapScan.class::cast)
+ .orElse(null);
+ long numNullsThreshold = scan == null ? -1 : getNumNullsThreshold(ctx);
for (Expression expr : exprs) {
- // remove generated `is not null`
- if (!(expr instanceof Not) || !((Not)
expr).isGeneratedIsNotNull()) {
+ // remove generated `is not null`, unless it is worth keeping for
the scan pruning
+ if (expr instanceof Not && ((Not) expr).isGeneratedIsNotNull()) {
Optional<Slot> notNullSlot = TypeUtils.isNotNull(expr);
- if (notNullSlot.isPresent()) {
+ if (notNullSlot.isPresent() && isWorthKeepingForScan(
Review Comment:
[P1] Preserve metadata MIN/MAX aggregation for aggregate-inferred filters
`InferAggNotNull` also produces this generated predicate. For `SELECT min(v)
FROM t` on a `DUP_KEYS` table with nullable `INT v`, cached `numNulls >
1000000`, and no valid cached exact simple-aggregate result, it produces this
reduced plan:
```text
Aggregate(min(v))
Filter(v IS NOT NULL [generated])
OlapScan(t)
```
This branch now retains the filter as an ordinary predicate. Previously
`EliminateNotNull` removed it, allowing `AggregateStrategies` to select
`PhysicalStorageLayerAggregate(MIN_MAX)`. Its generic storage-aggregate rules
only match `Aggregate(Scan)` or `Aggregate(Project(Scan))`, so the surviving
filter prevents that optimization. The BE consequently reads row data instead
of the two zone-map extrema per segment. This affects `MIN`/`MAX` under the
default threshold and can turn cheap aggregates on large nullable columns into
data scans. Preserve metadata-aggregate eligibility when deciding which
inferred filters to keep, and add a MIN/MAX plan regression.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -2168,6 +2172,13 @@ public boolean isEnableHboNonStrictMatchingMode() {
@VarAttrDef.VarAttr(name = REWRITE_OR_TO_IN_PREDICATE_THRESHOLD, fuzzy =
true)
private int rewriteOrToInPredicateThreshold = 2;
+ @VarAttrDef.VarAttr(name = KEEP_IS_NOT_NULL_NUM_NULLS_THRESHOLD, fuzzy =
true,
Review Comment:
[P2] Forward the threshold with queries replanned on the master
Please add `needForward = true` to this annotation. Both
`getForwardVariables()` and `setForwardedSessionVariables()` omit fields
without a forwarding/result flag, and the master plans forwarded SELECTs in a
fresh proxy context. For example, on a follower, set this threshold to `1` and
`force_forward_all_queries = true`, then join the two tables from the new
regression (two NULLs per `val`). The master receives no threshold override,
uses its default `1000000`, and eliminates both inferred scan predicates
instead of keeping them. Conversely, a session value of `-1` fails to disable
retention after forwarding when the columns have more than one million NULLs.
Add coverage that round-trips nondefault and negative values through the
forwarding map.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
