Skip to content

branch-4.1 [opt](fe) Bound not-null inference cost #63318#63914

Open
englefly wants to merge 1 commit into
apache:branch-4.1from
englefly:pick-63318-4.1
Open

branch-4.1 [opt](fe) Bound not-null inference cost #63318#63914
englefly wants to merge 1 commit into
apache:branch-4.1from
englefly:pick-63318-4.1

Conversation

@englefly
Copy link
Copy Markdown
Contributor

@englefly englefly commented May 31, 2026

pick #63318

@englefly englefly requested a review from yiguolei as a code owner May 31, 2026 03:54
@hello-stephen
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@englefly
Copy link
Copy Markdown
Contributor Author

run buildall

@morrySnow morrySnow changed the title branch-4.1 [opt](fe) Bound not-null inference cost (#63318) branch-4.1 [opt](fe) Bound not-null inference cost #63318 Jun 1, 2026
@morrySnow
Copy link
Copy Markdown
Contributor

run buildall

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 61.47% (67/109) 🎉
Increment coverage report
Complete coverage report

@englefly
Copy link
Copy Markdown
Contributor Author

englefly commented Jun 2, 2026

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one optimizer regression in the new not-null inference budget logic. Existing inline review context was empty, and there were no additional user focus points.

Critical checkpoints:

  • AGENTS.md Nereids expression checkpoint: not applicable; this PR does not add or change overloaded checkLegalityBeforeTypeCoercion methods.
  • Correctness: needs attention because cheap predicates can now be skipped based on unrelated predicate iteration order.
  • Tests: I attempted to run the focused FE tests with ./run-fe-ut.sh --run org.apache.doris.nereids.rules.rewrite.InferAggNotNullTest,org.apache.doris.nereids.rules.rewrite.InferJoinNotNullTest,org.apache.doris.nereids.rules.rewrite.InferFilterNotNullTest,org.apache.doris.nereids.rules.rewrite.EliminateNotNullTest, but the runner failed during generated-code setup because thirdparty/installed/bin/protoc is missing.

User focus response: no additional user-provided review focus was supplied.

mergedInputSlots.addAll(predicateInputSlots);
if (mergedInputSlots.size() > MAX_INFER_NOT_NULL_INPUT_SLOTS) {
return Optional.empty();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This global slot budget makes inference depend on the iteration order of the predicate collection. If an earlier cheap predicate uses 32 input slots, a later simple predicate like x = 1 reaches this branch and is skipped only because x would be the 33rd accumulated slot, so InferFilterNotNull will not infer x IS NOT NULL and EliminateNotNull will not remove an explicit redundant x IS NOT NULL. Several callers pass sets, so two equivalent predicate sets can get different not-null inference depending on iteration order. The budget should be applied per predicate, or the cumulative cap should not prevent independently cheap predicates from being considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants