Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
BiteThet merged PR #51163: URL: https://github.com/apache/doris/pull/51163 -- 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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
zclllyybb commented on code in PR #51163: URL: https://github.com/apache/doris/pull/51163#discussion_r2106405130 ## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java: ## @@ -41,13 +44,31 @@ public class Sha2 extends ScalarFunction FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT, IntegerType.INSTANCE), FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(StringType.INSTANCE, IntegerType.INSTANCE)); +private static final List validDigest = Lists.newArrayList(224, 256, 384, 512); Review Comment: ok. will fix in next 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: [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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris-website]
yiguolei merged PR #2406: URL: https://github.com/apache/doris-website/pull/2406 -- 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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
github-actions[bot] commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2903462219 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: [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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
github-actions[bot] commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2903462091 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: [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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
morrySnow commented on code in PR #51163: URL: https://github.com/apache/doris/pull/51163#discussion_r2103934094 ## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java: ## @@ -41,13 +44,31 @@ public class Sha2 extends ScalarFunction FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT, IntegerType.INSTANCE), FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(StringType.INSTANCE, IntegerType.INSTANCE)); +private static final List validDigest = Lists.newArrayList(224, 256, 384, 512); Review Comment: static variable's name should use UPPER_CASE name format -- 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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
doris-robot commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2900728694 TPC-H: Total hot run time: 34062 ms ``` machine: 'aliyun_ecs.c7a.8xlarge_32C64G' scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools Tpch sf100 test result on commit 814100c9dd68eab720159466bfbe2429e2f27e3a, data reload: false -- Round 1 -- q1 26036 527550255025 q2 2078270 182 182 q3 10400 1248705 705 q4 10227 1015552 552 q5 7537244523572357 q6 188 166 134 134 q7 945 736 638 638 q8 9327131812091209 q9 6965505052195050 q10 6881230418791879 q11 502 283 276 276 q12 368 363 228 228 q13 17794 367230693069 q14 236 235 207 207 q15 535 514 494 494 q16 445 434 376 376 q17 604 853 365 365 q18 7592719370237023 q19 1212959 599 599 q20 351 355 230 230 q21 3976317124552455 q22 1049103010091009 Total cold run time: 115248 ms Total hot run time: 34062 ms - Round 2, with runtime_filter_mode=off - q1 5170513350965096 q2 237 333 229 229 q3 2144267623272327 q4 1380181113661366 q5 4604445943834383 q6 213 171 130 130 q7 2042187917611761 q8 2560257025382538 q9 7191716271937162 q10 2977321327532753 q11 583 522 494 494 q12 686 760 582 582 q13 3556391733463346 q14 287 280 267 267 q15 533 490 495 490 q16 455 483 443 443 q17 1171153213941394 q18 7784762873377337 q19 831 850 912 850 q20 2017205018071807 q21 4726430943264309 q22 10911024990 990 Total cold run time: 52238 ms Total hot run time: 50054 ms ``` -- 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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
doris-robot commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2900775149 ClickBench: Total hot run time: 29.04 s ``` machine: 'aliyun_ecs.c7a.8xlarge_32C64G' scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools ClickBench test result on commit 814100c9dd68eab720159466bfbe2429e2f27e3a, data reload: false query1 0.030.030.04 query2 0.120.100.10 query3 0.250.200.20 query4 1.580.200.20 query5 0.440.430.45 query6 1.560.650.66 query7 0.020.020.02 query8 0.040.040.04 query9 0.570.510.52 query10 0.570.570.57 query11 0.150.110.11 query12 0.150.120.11 query13 0.620.600.60 query14 0.780.790.81 query15 0.870.850.85 query16 0.390.390.38 query17 0.991.051.04 query18 0.210.210.21 query19 1.891.851.82 query20 0.010.010.01 query21 15.39 0.930.55 query22 0.741.110.66 query23 15.03 1.380.63 query24 7.121.810.82 query25 0.480.200.13 query26 0.610.160.13 query27 0.060.050.04 query28 9.430.880.45 query29 12.51 3.893.26 query30 0.270.100.07 query31 2.850.590.40 query32 3.230.540.46 query33 2.953.063.05 query34 15.70 5.104.43 query35 4.514.494.48 query36 0.660.490.49 query37 0.090.060.06 query38 0.060.040.04 query39 0.030.030.02 query40 0.170.130.12 query41 0.070.030.03 query42 0.040.020.03 query43 0.030.030.03 Total cold run time: 103.27 s Total hot run time: 29.04 s ``` -- 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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
doris-robot commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2900760926 TPC-DS: Total hot run time: 186040 ms ``` machine: 'aliyun_ecs.c7a.8xlarge_32C64G' scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools TPC-DS sf100 test result on commit 814100c9dd68eab720159466bfbe2429e2f27e3a, data reload: false query1 1022478 482 478 query2 6534184518821845 query3 6751230 224 224 query4 26335 23656 23430 23430 query5 4362654 449 449 query6 288 200 190 190 query7 4613485 289 289 query8 288 238 232 232 query9 8628261226102610 query10 456 344 264 264 query11 15244 15519 14944 14944 query12 167 118 111 111 query13 1656529 415 415 query14 8580607360456045 query15 214 187 176 176 query16 7092609 430 430 query17 997 730 576 576 query18 1951378 287 287 query19 188 190 167 167 query20 118 125 117 117 query21 211 124 103 103 query22 4046416841394139 query23 34009 32861 33153 32861 query24 8380238023912380 query25 521 464 387 387 query26 1204277 156 156 query27 2742495 343 343 query28 4345212121092109 query29 698 561 424 424 query30 277 220 192 192 query31 923 856 779 779 query32 72 63 66 63 query33 549 368 320 320 query34 815 840 541 541 query35 790 820 723 723 query36 932 983 915 915 query37 117 101 75 75 query38 4114418940234023 query39 1484140013811381 query40 213 119 105 105 query41 58 57 56 56 query42 120 114 117 114 query43 516 505 471 471 query44 1310830 807 807 query45 174 172 170 170 query46 832 1021635 635 query47 1784178917391739 query48 403 415 310 310 query49 795 554 456 456 query50 640 705 433 433 query51 4116422140754075 query52 112 101 102 101 query53 223 256 186 186 query54 601 582 537 537 query55 85 83 88 83 query56 309 305 302 302 query57 1128113010831083 query58 265 269 254 254 query59 2637271025942594 query60 311 322 303 303 query61 128 130 124 124 query62 800 744 649 649 query63 226 186 189 186 query64 43001089781 781 query65 4281423542414235 query66 1090429 326 326 query67 16102 15449 15598 15449 query68 7788885 522 522 query69 551 326 285 285 query70 1198114010771077 query71 428 339 316 316 query72 6138474649514746 query73 687 649 346 346 query74 8942909186668666 query75 3209321526892689 query76 34191184776 776 query77 468 375 399 375 query78 10043 10104 92359235 query79 2646812 593 593 query80 575 494 463 463 query81 498 251 223 223 query82 444 123 96 96 query83 284 256 235 235 query84 304 99 81 81 query85 754 352 358 352 query86 342 334 301 301 query87 4474443943874387 query88 3644226822472247 query89 404 321 286 286 query90 1924207 204 204 query91 140 145 109 109 query92 85 65 58 58 query93 2095925 575 575 query94 726 411 308 308 query95 368 289 281 281 query96 492 581 278 278 query97 2715277926902690 query98 230 217 203 203 query99 1411140412801280 Total cold run time: 272698 ms Total hot run time: 186040 ms ``` -- 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
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
zclllyybb commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2900618842 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: [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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
zclllyybb commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2900490737 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: [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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
morrySnow commented on code in PR #51163:
URL: https://github.com/apache/doris/pull/51163#discussion_r2102074527
##
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##
@@ -48,6 +51,22 @@ public Sha2(Expression arg0, Expression arg1) {
super("sha2", arg0, arg1);
}
+@Override
+public void checkLegalityBeforeTypeCoercion() {
+checkLegalityAfterRewrite();
+}
+
+@Override
+public void checkLegalityAfterRewrite() {
+if ((children.size() != 2) || !(child(1) instanceof
IntegerLikeLiteral)) {
+throw new AnalysisException("sha2 needs two params, and the second
is must be a integer constant: " + this.toSql());
+}
+final int constParam = ((IntegerLikeLiteral) child(1)).getIntValue();
+if (!Lists.newArrayList(224, 256, 384, 512).contains(constParam)) {
Review Comment:
please use a static list for Lists.newArrayList(224, 256, 384, 512)
##
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##
@@ -48,6 +51,22 @@ public Sha2(Expression arg0, Expression arg1) {
super("sha2", arg0, arg1);
}
+@Override
+public void checkLegalityBeforeTypeCoercion() {
+checkLegalityAfterRewrite();
+}
+
+@Override
+public void checkLegalityAfterRewrite() {
+if ((children.size() != 2) || !(child(1) instanceof
IntegerLikeLiteral)) {
+throw new AnalysisException("sha2 needs two params, and the second
is must be a integer constant: " + this.toSql());
+}
+final int constParam = ((IntegerLikeLiteral) child(1)).getIntValue();
+if (!Lists.newArrayList(224, 256, 384, 512).contains(constParam)) {
+throw new AnalysisException("sha2 functions only support digest
length of 224/256/384/512");
Review Comment:
use List to string to avoid exception msg is not same with if condiiton
##
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sha2.java:
##
@@ -48,6 +51,22 @@ public Sha2(Expression arg0, Expression arg1) {
super("sha2", arg0, arg1);
}
+@Override
+public void checkLegalityBeforeTypeCoercion() {
+checkLegalityAfterRewrite();
+}
+
+@Override
+public void checkLegalityAfterRewrite() {
+if ((children.size() != 2) || !(child(1) instanceof
IntegerLikeLiteral)) {
+throw new AnalysisException("sha2 needs two params, and the second
is must be a integer constant: " + this.toSql());
Review Comment:
constant is not same with literal, the log is not reflect the if condition
conrrectly
--
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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
hello-stephen commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2900482277 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). 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? -- 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]
Re: [PR] [Fix](function) Add lost check of function SHA2 in nereids [doris]
zclllyybb commented on PR #51163: URL: https://github.com/apache/doris/pull/51163#issuecomment-2900480507 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: [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]
