You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "morningman (via GitHub)" <gi...@apache.org> on 2024/03/14 10:22:27 UTC
[PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
morningman opened a new pull request, #32253:
URL: https://github.com/apache/doris/pull/32253
## Proposed changes
Previously, for JNI scanner, BE will assemble a `push_down_predicates` from `colname_to_value_range`
and pass it to JNI scanner. and JNI scanner will determine how to use it to do the predicate pushdown.
But actually, this `push_down_predicates` is never used.
This PR removes the `push_down_predicates`.
This will also avoid BE crash when encountering decimal type predicate(a bug, not fixed, but skip it)
## Further comments
Actually the predicate passed to JNI scanner can be assembled and serialized on FE side, and pass to BE
then pass to JNI scanner directly, just like what PaimonJniScanner do(see `paimon_predicate`).
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997169745
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997121005
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1998699090
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997140193
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #32253:
URL: https://github.com/apache/doris/pull/32253
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997111366
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997170997
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997180892
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on code in PR #32253:
URL: https://github.com/apache/doris/pull/32253#discussion_r1525707820
##########
fe/be-java-extensions/java-common/src/main/java/org/apache/doris/common/jni/JniScanner.java:
##########
@@ -33,6 +33,14 @@ public abstract class JniScanner {
protected VectorTable vectorTable;
protected String[] fields;
protected ColumnType[] types;
+ @Deprecated
+ // This predicate is from BE, but no used.
+ // TODO: actually, we can generate the predicate for JNI scanner in FE's planner,
+ // then serialize it to BE, and BE pass it to JNI scanner directly.
+ // NO need to use this intermediate expression, because each JNI scanner has its
+ // own predicate expression format.
+ // For example, Paimon use "PaimonScannerUtils.decodeStringToObject(paimonPredicate)"
+ // to deserialize the predicate string to PaimonPredicate object.
protected ScanPredicate[] predicates;
Review Comment:
I leave the related class.
Maybe we can delete it when refactor this logic.
We still need a predicate pushdown framework
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1998699123
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997238505
<details>
<summary>TPC-H: <b>Total hot run time: 37924 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 04b5dfdeff1a2e1ca206f5910cfbf9252cd30a5b, data reload: false
------ Round 1 ----------------------------------
q1 17665 4143 4133 4133
q2 2029 143 141 141
q3 10613 1113 904 904
q4 7435 732 692 692
q5 7474 2562 2525 2525
q6 204 119 123 119
q7 1186 822 783 783
q8 9334 1990 1969 1969
q9 7068 6429 6388 6388
q10 8502 3439 3666 3439
q11 438 232 215 215
q12 632 306 296 296
q13 17790 2842 2849 2842
q14 273 241 246 241
q15 506 454 454 454
q16 498 405 391 391
q17 943 544 531 531
q18 7230 6464 6317 6317
q19 3062 1440 1370 1370
q20 550 283 284 283
q21 6291 3584 3652 3584
q22 375 307 309 307
Total cold run time: 110098 ms
Total hot run time: 37924 ms
----- Round 2, with runtime_filter_mode=off -----
q1 4158 4130 4081 4081
q2 319 220 222 220
q3 2946 2843 2815 2815
q4 1873 1554 1537 1537
q5 5237 5219 5281 5219
q6 191 122 117 117
q7 2200 1861 1858 1858
q8 3159 3256 3257 3256
q9 8579 8569 8563 8563
q10 3709 3757 3726 3726
q11 545 449 440 440
q12 744 606 578 578
q13 16935 2883 2867 2867
q14 282 253 270 253
q15 491 458 450 450
q16 455 415 416 415
q17 1742 1495 1449 1449
q18 7607 7213 7259 7213
q19 1653 1529 1574 1529
q20 1924 1734 1709 1709
q21 4914 4802 4865 4802
q22 541 438 463 438
Total cold run time: 70204 ms
Total hot run time: 53535 ms
```
</details>
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #32253:
URL: https://github.com/apache/doris/pull/32253#issuecomment-1997255520
TeamCity be ut coverage result:
Function Coverage: 34.99% (8573/24499)
Line Coverage: 26.73% (69457/259799)
Region Coverage: 26.01% (36074/138687)
Branch Coverage: 22.97% (18421/80204)
Coverage Report: http://coverage.selectdb-in.cc/coverage/04b5dfdeff1a2e1ca206f5910cfbf9252cd30a5b_04b5dfdeff1a2e1ca206f5910cfbf9252cd30a5b/report/index.html
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](jni) remove 'push_down_predicates' and fix BE crash with decimal predicate [doris]
Posted by "BePPPower (via GitHub)" <gi...@apache.org>.
BePPPower commented on code in PR #32253:
URL: https://github.com/apache/doris/pull/32253#discussion_r1525669751
##########
fe/be-java-extensions/java-common/src/main/java/org/apache/doris/common/jni/JniScanner.java:
##########
@@ -33,6 +33,14 @@ public abstract class JniScanner {
protected VectorTable vectorTable;
protected String[] fields;
protected ColumnType[] types;
+ @Deprecated
+ // This predicate is from BE, but no used.
+ // TODO: actually, we can generate the predicate for JNI scanner in FE's planner,
+ // then serialize it to BE, and BE pass it to JNI scanner directly.
+ // NO need to use this intermediate expression, because each JNI scanner has its
+ // own predicate expression format.
+ // For example, Paimon use "PaimonScannerUtils.decodeStringToObject(paimonPredicate)"
+ // to deserialize the predicate string to PaimonPredicate object.
protected ScanPredicate[] predicates;
Review Comment:
Is this member variable still in use? Why not delete it?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org