Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix] Clear probe RF whose probe expr contains dict mapping expr #50690

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

ZiheLiu
Copy link
Contributor

@ZiheLiu ZiheLiu commented Sep 4, 2024

Why I'm doing:

If the equi-join predicate is derived from an expression evaluated on a column with low cardinality, a corresponding runtime filter will be generated on the OlapScanNode. The probe expression for this filter is a DictMappingExpr.

Except for the main branch, this DictMappingExpr in the probe expression is not rewritten, leading to a crash when evaluated using a DictID column.

*** Aborted at 1725338148 (unix time) try "date -d @1725338148" if you are using GNU date ***
PC: @          0x2dc0ce0 starrocks::memequal()
*** SIGSEGV (@0x7) received by PID 167210 (TID 0x7f57b5d00700) from PID 7; stack trace: ***
    @          0x5c32442 google::(anonymous namespace)::FailureSignalHandler()
    @     0x7f58f08c7630 (unknown)
    @          0x2dc0ce0 starrocks::memequal()
    @          0x3a6b20a starrocks::vectorized::UnpackConstColumnBinaryFunction<>::evaluate<>()
    @          0x3a6b61c starrocks::vectorized::VectorizedBinaryPredicate<>::evaluate_checked()
    @          0x4045b19 starrocks::vectorized::DictMappingExpr::evaluate_checked()
    @          0x3b7df24 starrocks::vectorized::VectorizedCaseExpr<>::evaluate_no_case()
    @          0x3b84e0d starrocks::vectorized::VectorizedCaseExpr<>::evaluate_checked()
    @          0x394a3b3 starrocks::ExprContext::evaluate()
    @          0x4133af0 starrocks::vectorized::RuntimeFilterProbeCollector::update_selectivity()
    @          0x4134caf starrocks::vectorized::RuntimeFilterProbeCollector::evaluate()
    @          0x2dd4506 starrocks::pipeline::Operator::eval_runtime_bloom_filters()
    @          0x2ddab55 starrocks::pipeline::ScanOperator::pull_chunk()
    @          0x2db7ed0 starrocks::pipeline::PipelineDriver::process()
    @          0x5207fca starrocks::pipeline::GlobalDriverExecutor::_worker_thread()
    @          0x4bea3a2 starrocks::ThreadPool::dispatch_thread()
    @          0x4be4e3a starrocks::Thread::supervise_thread()
    @     0x7f58f08bfea5 start_thread
    @     0x7f58efedab0d __clone
    @                0x0 (unknown)

Plan:

  3:Decode
  |  <dict id 38> : <string id 2>
  |  cardinality: 1
  |  probe runtime filters:
  |  - filter_id = 1, probe_expr = (2: P_NAME)
  |  column statistics: 
  |  * P_NAME-->[-Infinity, Infinity, 0.0, 1.0, 1.0] UNKNOWN
  |  * P_BRAND-->[-Infinity, Infinity, 0.0, 1.0, 1.0] UNKNOWN
  |  * cast-->[-Infinity, Infinity, 0.0, 16.0, 3.0] ESTIMATE
  |  
  2:Project
  |  output columns:
  |  12 <-> CASE WHEN DictDecode(38: P_NAME, [<place-holder> = 'a']) THEN 'a1' WHEN DictDecode(39: P_BRAND, [<place-holder> = 'b']) THEN 'b1' ELSE 'c1' END
  |  38 <-> [38: P_NAME, INT, false]
  |  cardinality: 1
  |  column statistics: 
  |  * cast-->[-Infinity, Infinity, 0.0, 16.0, 3.0] ESTIMATE
  |  
  1:OlapScanNode
     table: part_v2, rollup: part_v2
     preAggregation: on
     dict_col=P_NAME,P_BRAND
     partitionsRatio=1/1, tabletsRatio=10/10
     tabletList=14325,14327,14329,14331,14333,14335,14337,14339,14341,14343
     actualRows=0, avgRowSize=18.0
     cardinality: 1
     probe runtime filters:
     - filter_id = 0, probe_expr = (CASE WHEN DictDecode(38: P_NAME, [<place-holder> = 'a']) THEN 'a1' WHEN DictDecode(39: P_BRAND, [<place-holder> = 'b']) THEN 'b1' ELSE 'c1' END)

What I'm doing:

To address this case, a check is performed in the ProjectNode. If an expression contains a DictMappingExpr, the RF will be no longer pushed down to its children.

Additionally, both FE and BE have implemented a fallback strategy to remove all probe runtime filters that include a DictMappingExpr in the probe expression.

Plan after this PR:

  3:Decode
  |  <dict id 38> : <string id 2>
  |  cardinality: 1
  |  probe runtime filters:
  |  - filter_id = 1, probe_expr = (2: P_NAME)
  |  column statistics: 
  |  * P_NAME-->[-Infinity, Infinity, 0.0, 1.0, 1.0] UNKNOWN
  |  * P_BRAND-->[-Infinity, Infinity, 0.0, 1.0, 1.0] UNKNOWN
  |  * cast-->[-Infinity, Infinity, 0.0, 16.0, 3.0] ESTIMATE
  |  
  2:Project
  |  output columns:
  |  12 <-> CASE WHEN DictDecode(38: P_NAME, [<place-holder> = 'a']) THEN 'a1' WHEN DictDecode(39: P_BRAND, [<place-holder> = 'b']) THEN 'b1' ELSE 'c1' END
  |  38 <-> [38: P_NAME, INT, false]
  |  cardinality: 1
  |  probe runtime filters:
  |  - filter_id = 0, probe_expr = (<slot 12>)
  |  column statistics: 
  |  * cast-->[-Infinity, Infinity, 0.0, 16.0, 3.0] ESTIMATE
  |  
  1:OlapScanNode
     table: part_v2, rollup: part_v2
     preAggregation: on
     dict_col=P_NAME,P_BRAND
     partitionsRatio=0/1, tabletsRatio=0/0
     tabletList=
     actualRows=0, avgRowSize=18.0
     cardinality: 1
     column statistics: 
     * P_NAME-->[-Infinity, Infinity, 0.0, 1.0, 1.0] UNKNOWN
     * P_BRAND-->[-Infinity, Infinity, 0.0, 1.0, 1.0] UNKNOWN
     * cast-->[-Infinity, Infinity, 0.0, 16.0, 3.0] ESTIMATE
     * 

Fixes #issue

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.3
    • 3.2
    • 3.1
    • 3.0
    • 2.5

Signed-off-by: zihe.liu <[email protected]>
Copy link

sonarcloud bot commented Sep 4, 2024

Copy link

github-actions bot commented Sep 4, 2024

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

github-actions bot commented Sep 4, 2024

[FE Incremental Coverage Report]

pass : 17 / 17 (100.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/sql/plan/PlanFragmentBuilder.java 1 1 100.00% []
🔵 com/starrocks/analysis/Expr.java 4 4 100.00% []
🔵 com/starrocks/planner/PlanFragment.java 10 10 100.00% []
🔵 com/starrocks/planner/ProjectNode.java 2 2 100.00% []

Copy link

github-actions bot commented Sep 4, 2024

[BE Incremental Coverage Report]

pass : 12 / 12 (100.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 be/src/exprs/runtime_filter_bank.cpp 12 12 100.00% []

@ZiheLiu ZiheLiu merged commit a810948 into StarRocks:main Sep 5, 2024
52 of 53 checks passed
@ZiheLiu ZiheLiu deleted the fix/main/low_cardinality_rf branch September 5, 2024 02:18
Copy link

github-actions bot commented Sep 5, 2024

@Mergifyio backport branch-3.3

@github-actions github-actions bot removed the 3.3 label Sep 5, 2024
Copy link

github-actions bot commented Sep 5, 2024

@Mergifyio backport branch-3.2

@github-actions github-actions bot removed the 3.2 label Sep 5, 2024
Copy link

github-actions bot commented Sep 5, 2024

@Mergifyio backport branch-3.1

@github-actions github-actions bot removed the 3.1 label Sep 5, 2024
Copy link

github-actions bot commented Sep 5, 2024

@Mergifyio backport branch-3.0

@github-actions github-actions bot removed the 3.0 label Sep 5, 2024
mergify bot pushed a commit that referenced this pull request Sep 5, 2024
…50690)

Signed-off-by: zihe.liu <[email protected]>
(cherry picked from commit a810948)

# Conflicts:
#	fe/fe-core/src/main/java/com/starrocks/planner/ProjectNode.java
#	fe/fe-core/src/test/java/com/starrocks/sql/plan/LowCardinalityTest.java
mergify bot pushed a commit that referenced this pull request Sep 5, 2024
…50690)

Signed-off-by: zihe.liu <[email protected]>
(cherry picked from commit a810948)

# Conflicts:
#	be/src/exprs/runtime_filter_bank.cpp
#	fe/fe-core/src/main/java/com/starrocks/planner/ProjectNode.java
mergify bot pushed a commit that referenced this pull request Sep 5, 2024
…50690)

Signed-off-by: zihe.liu <[email protected]>
(cherry picked from commit a810948)

# Conflicts:
#	be/src/exprs/runtime_filter_bank.cpp
#	fe/fe-core/src/main/java/com/starrocks/planner/ProjectNode.java
mergify bot pushed a commit that referenced this pull request Sep 5, 2024
…50690)

Signed-off-by: zihe.liu <[email protected]>
(cherry picked from commit a810948)

# Conflicts:
#	be/src/exprs/vectorized/runtime_filter_bank.cpp
#	fe/fe-core/src/main/java/com/starrocks/planner/ProjectNode.java
ZiheLiu added a commit to ZiheLiu/starrocks that referenced this pull request Sep 5, 2024
ZiheLiu added a commit to ZiheLiu/starrocks that referenced this pull request Sep 5, 2024
ZiheLiu added a commit to ZiheLiu/starrocks that referenced this pull request Sep 5, 2024
ZiheLiu added a commit to ZiheLiu/starrocks that referenced this pull request Sep 5, 2024
ZiheLiu added a commit to ZiheLiu/starrocks that referenced this pull request Sep 5, 2024
satanson pushed a commit that referenced this pull request Sep 5, 2024
satanson pushed a commit that referenced this pull request Sep 5, 2024
satanson pushed a commit that referenced this pull request Sep 5, 2024
satanson pushed a commit that referenced this pull request Sep 5, 2024
satanson pushed a commit that referenced this pull request Sep 5, 2024
xiangguangyxg pushed a commit to xiangguangyxg/starrocks that referenced this pull request Sep 6, 2024
renzhimin7 pushed a commit to renzhimin7/starrocks that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants