You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "mrhhsg (via GitHub)" <gi...@apache.org> on 2026/05/16 02:41:09 UTC
[PR] [fix](json) Fix strict json validation and typed extraction [doris]
mrhhsg opened a new pull request, #63309:
URL: https://github.com/apache/doris/pull/63309
### What problem does this PR solve?
Issue Number: DORIS-25567, DORIS-25568
Problem Summary:
- Fix `json_valid` accepting incomplete JSON or inputs with trailing non-JSON content, such as `not json` and `null junk`.
- Fix typed `jsonb_extract_*` functions returning implicitly cast values when the JSON value type does not match the requested type. These functions now return `NULL` on type mismatch.
Root cause:
- The simdjson parser path accepted partial top-level JSON without verifying that the parsed raw JSON consumed the full input.
- Nereids rewrote typed `jsonb_extract_*` calls to `cast(jsonb_extract(...) as type)`, which allowed implicit casts instead of strict JSON type checks.
### Release note
Fix `json_valid` and typed `jsonb_extract_*` strict JSON type behavior.
### Check List (For Author)
- Test: Unit Test / Regression test
- `./run-be-ut.sh --run --filter=JsonBinaryValueTest.TestValidation:FunctionJsonbTEST.JsonValidStrictTest:FunctionJsonbTEST.JsonbExtractStrictTypedTest`
- `./build.sh --fe`
- `./build.sh --be`
- `env -u HTTP_PROXY -u HTTPS_PROXY -u http_proxy -u https_proxy -u ALL_PROXY -u all_proxy ./run-regression-test.sh --run -d jsonb_p0 -s test_jsonb_strict_type_extract_and_valid`
- Behavior changed: Yes. `json_valid` rejects incomplete/trailing JSON input and typed `jsonb_extract_*` returns `NULL` on type mismatch.
- Does this need documentation: No
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4496008176
<details>
<summary>TPC-DS: <b>Total hot run time: 169591 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 79bfbef96395e80cd3c5a7ebcaba6915ae800bbd, data reload: false
query5 4358 671 530 530
query6 332 219 206 206
query7 4269 591 305 305
query8 337 226 231 226
query9 8804 4016 4010 4010
query10 441 347 302 302
query11 5737 2483 2225 2225
query12 182 130 126 126
query13 1306 624 438 438
query14 6086 5420 5048 5048
query14_1 4354 4316 4315 4315
query15 212 204 180 180
query16 1008 493 450 450
query17 1129 723 596 596
query18 2552 486 354 354
query19 216 204 159 159
query20 145 135 130 130
query21 216 135 117 117
query22 13603 13657 13357 13357
query23 17412 16322 16060 16060
query23_1 16191 16271 16098 16098
query24 7444 1741 1313 1313
query24_1 1267 1294 1292 1292
query25 555 467 434 434
query26 1363 333 170 170
query27 2665 549 327 327
query28 4536 1941 1943 1941
query29 990 610 493 493
query30 298 240 197 197
query31 1117 1052 940 940
query32 101 74 73 73
query33 539 355 289 289
query34 1170 1143 634 634
query35 766 812 662 662
query36 1327 1355 1160 1160
query37 162 106 91 91
query38 3220 3089 3055 3055
query39 932 934 913 913
query39_1 880 908 892 892
query40 228 142 124 124
query41 66 64 61 61
query42 115 111 110 110
query43 330 328 290 290
query44
query45 208 202 194 194
query46 1118 1221 739 739
query47 2365 2336 2234 2234
query48 405 428 289 289
query49 634 492 380 380
query50 1043 346 258 258
query51 4296 4304 4246 4246
query52 109 106 93 93
query53 254 291 211 211
query54 315 273 260 260
query55 92 89 84 84
query56 298 321 312 312
query57 1422 1405 1343 1343
query58 317 291 287 287
query59 1585 1650 1449 1449
query60 335 327 330 327
query61 181 178 178 178
query62 677 633 549 549
query63 260 205 212 205
query64 2472 872 700 700
query65
query66 1724 484 369 369
query67 30156 29465 29959 29465
query68
query69 485 359 317 317
query70 1008 996 912 912
query71 316 285 283 283
query72 3221 2987 2412 2412
query73 836 754 417 417
query74 5101 5041 4731 4731
query75 2690 2598 2273 2273
query76 2292 1168 800 800
query77 410 411 331 331
query78 12186 12111 11703 11703
query79 1458 1094 765 765
query80 1321 553 451 451
query81 526 278 246 246
query82 1200 157 120 120
query83 352 273 245 245
query84 274 143 113 113
query85 986 544 464 464
query86 456 339 310 310
query87 3489 3368 3233 3233
query88 3520 2638 2626 2626
query89 472 386 338 338
query90 1889 184 186 184
query91 181 174 142 142
query92 84 78 75 75
query93 1552 1373 881 881
query94 724 341 310 310
query95 688 475 355 355
query96 1049 749 334 334
query97 2688 2688 2610 2610
query98 240 230 237 230
query99 1108 1137 987 987
Total cold run time: 254949 ms
Total hot run time: 169591 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4469560741
<details>
<summary>TPC-DS: <b>Total hot run time: 169127 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit eaf2f141802181bd1c09fbe0dbe3b2a27414025f, data reload: false
query5 4294 668 524 524
query6 335 225 201 201
query7 4253 578 303 303
query8 319 228 236 228
query9 8810 4041 4032 4032
query10 470 341 287 287
query11 5772 2388 2252 2252
query12 184 126 128 126
query13 1306 594 437 437
query14 6009 5400 5122 5122
query14_1 4363 4394 4322 4322
query15 215 207 185 185
query16 1015 460 447 447
query17 1129 709 624 624
query18 2650 508 361 361
query19 211 189 179 179
query20 135 130 126 126
query21 221 140 115 115
query22 13612 13557 13331 13331
query23 17255 16317 15944 15944
query23_1 16123 16202 16149 16149
query24 7397 1762 1294 1294
query24_1 1276 1281 1304 1281
query25 538 481 407 407
query26 1329 304 169 169
query27 2717 584 336 336
query28 4410 2007 1964 1964
query29 981 607 526 526
query30 297 235 197 197
query31 1096 1066 934 934
query32 83 75 75 75
query33 536 357 288 288
query34 1162 1151 623 623
query35 758 783 678 678
query36 1352 1369 1137 1137
query37 151 102 90 90
query38 3207 3130 3045 3045
query39 928 910 898 898
query39_1 868 898 880 880
query40 224 151 127 127
query41 64 63 60 60
query42 115 112 108 108
query43 327 330 283 283
query44
query45 208 197 193 193
query46 1074 1255 717 717
query47 2274 2358 2193 2193
query48 406 416 281 281
query49 631 488 382 382
query50 955 341 258 258
query51 4280 4267 4251 4251
query52 104 104 95 95
query53 260 292 210 210
query54 324 270 263 263
query55 94 89 86 86
query56 296 324 304 304
query57 1389 1409 1312 1312
query58 314 269 265 265
query59 1541 1618 1419 1419
query60 338 328 320 320
query61 190 182 177 177
query62 676 624 556 556
query63 251 206 206 206
query64 2459 851 707 707
query65
query66 1700 494 358 358
query67 29985 29379 29848 29379
query68
query69 462 351 315 315
query70 1059 968 984 968
query71 311 288 325 288
query72 3065 2744 2354 2354
query73 849 784 425 425
query74 5033 4928 4695 4695
query75 2650 2592 2262 2262
query76 2262 1148 813 813
query77 406 410 324 324
query78 12257 12200 11629 11629
query79 1228 1044 708 708
query80 595 553 490 490
query81 451 277 240 240
query82 228 161 123 123
query83 271 284 256 256
query84 262 136 118 118
query85 841 541 470 470
query86 362 331 312 312
query87 3406 3378 3213 3213
query88 3573 2701 2680 2680
query89 424 390 336 336
query90 2165 178 191 178
query91 180 167 138 138
query92 81 79 71 71
query93 1390 1534 834 834
query94 538 356 325 325
query95 659 382 433 382
query96 1049 802 338 338
query97 2665 2679 2570 2570
query98 245 233 236 233
query99 1107 1097 949 949
Total cold run time: 251503 ms
Total hot run time: 169127 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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4469511264
/review
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4620980323
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/27)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46_merge_fe/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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4495850377
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/89)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_79bfbef96395e80cd3c5a7ebcaba6915ae800bbd_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_79bfbef96395e80cd3c5a7ebcaba6915ae800bbd_merge_fe/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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4505962997
/review
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4484985605
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/321)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_02d620ae9a0606d7da74eb54d45a1697a6cfb72c_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_02d620ae9a0606d7da74eb54d45a1697a6cfb72c_merge_fe/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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4503993959
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4504691741
<details>
<summary>TPC-H: <b>Total hot run time: 31093 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 8257d121b17ba41337a4cb57e6261f598e19ae46, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17712 3927 4049 3927
q2 q3 10805 1402 831 831
q4 4704 467 352 352
q5 7763 2263 2120 2120
q6 382 178 138 138
q7 948 776 654 654
q8 9598 1698 1649 1649
q9 7086 4891 4883 4883
q10 6429 2069 1793 1793
q11 431 270 238 238
q12 640 424 295 295
q13 18104 3426 2752 2752
q14 262 256 230 230
q15 q16 820 775 711 711
q17 951 902 873 873
q18 6877 5752 5483 5483
q19 1169 1307 1109 1109
q20 521 407 265 265
q21 5487 2572 2486 2486
q22 442 361 304 304
Total cold run time: 101131 ms
Total hot run time: 31093 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4304 4161 4164 4161
q2 q3 4530 4935 4347 4347
q4 2086 2183 1394 1394
q5 4432 4277 4277 4277
q6 230 246 277 246
q7 2128 1860 1612 1612
q8 2461 2114 2083 2083
q9 7667 7742 7696 7696
q10 4559 4454 4032 4032
q11 774 416 365 365
q12 720 739 526 526
q13 3356 3606 3070 3070
q14 304 321 294 294
q15 q16 715 742 672 672
q17 1328 1307 1319 1307
q18 7943 7367 7019 7019
q19 1117 1118 1130 1118
q20 2229 2212 1939 1939
q21 5365 4692 4589 4589
q22 522 451 421 421
Total cold run time: 56770 ms
Total hot run time: 51168 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4632395409
<details>
<summary>TPC-DS: <b>Total hot run time: 169653 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit f6a987323ca9c51ca1901b7ae6f933763fb3faa2, data reload: false
query5 4331 630 501 501
query6 445 200 175 175
query7 4845 574 309 309
query8 385 216 199 199
query9 8761 4045 4039 4039
query10 459 324 258 258
query11 5920 2379 2139 2139
query12 154 104 97 97
query13 1258 601 429 429
query14 6417 5425 5082 5082
query14_1 4470 4463 4776 4463
query15 209 199 173 173
query16 984 485 432 432
query17 1083 727 596 596
query18 2495 464 350 350
query19 214 193 149 149
query20 115 112 108 108
query21 224 141 119 119
query22 13722 13504 13279 13279
query23 17370 16522 16252 16252
query23_1 16224 16371 16345 16345
query24 7555 1779 1307 1307
query24_1 1314 1346 1326 1326
query25 567 478 416 416
query26 1303 317 170 170
query27 2661 579 359 359
query28 4416 2054 2056 2054
query29 1102 633 495 495
query30 315 236 207 207
query31 1115 1094 965 965
query32 112 67 63 63
query33 541 336 266 266
query34 1166 1164 659 659
query35 762 772 690 690
query36 1366 1383 1254 1254
query37 153 103 90 90
query38 3219 3108 3051 3051
query39 952 925 893 893
query39_1 872 881 880 880
query40 229 121 103 103
query41 67 66 67 66
query42 94 95 95 95
query43 321 319 285 285
query44
query45 194 187 181 181
query46 1144 1200 765 765
query47 2347 2326 2222 2222
query48 384 383 285 285
query49 616 471 347 347
query50 992 338 260 260
query51 4340 4330 4197 4197
query52 89 92 77 77
query53 244 267 190 190
query54 263 219 201 201
query55 86 78 72 72
query56 250 223 217 217
query57 1419 1367 1301 1301
query58 243 218 210 210
query59 1571 1637 1425 1425
query60 277 253 230 230
query61 154 161 151 151
query62 683 656 585 585
query63 241 183 184 183
query64 2571 797 634 634
query65
query66 1770 457 342 342
query67 29731 29673 29474 29474
query68
query69 420 312 274 274
query70 941 972 926 926
query71 301 218 204 204
query72 2897 2710 2453 2453
query73 841 740 397 397
query74 5101 4974 4779 4779
query75 2694 2579 2231 2231
query76 2320 1163 770 770
query77 347 393 285 285
query78 12401 12332 11825 11825
query79 1471 989 764 764
query80 597 471 380 380
query81 453 282 258 258
query82 565 151 119 119
query83 357 278 246 246
query84 271 143 109 109
query85 860 567 436 436
query86 360 295 291 291
query87 3380 3385 3201 3201
query88 3687 2772 2727 2727
query89 420 376 332 332
query90 1980 185 181 181
query91 176 175 137 137
query92 65 63 56 56
query93 1501 1374 865 865
query94 520 368 324 324
query95 706 463 367 367
query96 1091 783 351 351
query97 2695 2669 2573 2573
query98 213 202 214 202
query99 1140 1174 1017 1017
Total cold run time: 250970 ms
Total hot run time: 169653 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4633814448
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/237)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge_fe/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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4498523207
/review
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4499884336
<details>
<summary>TPC-H: <b>Total hot run time: 30941 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 a92ea1199e9f133540f9e9c62a72a61629dd687e, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17788 3853 3817 3817
q2 q3 10831 1352 815 815
q4 4688 464 347 347
q5 7608 2237 2099 2099
q6 292 172 141 141
q7 958 776 639 639
q8 9348 1805 1673 1673
q9 6714 4929 4873 4873
q10 6450 2080 1770 1770
q11 436 279 239 239
q12 692 427 300 300
q13 18175 3397 2800 2800
q14 257 259 233 233
q15 q16 816 764 700 700
q17 966 975 978 975
q18 6762 5702 5471 5471
q19 1152 1404 1091 1091
q20 542 409 271 271
q21 5761 2568 2386 2386
q22 428 359 301 301
Total cold run time: 100664 ms
Total hot run time: 30941 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4171 4089 4091 4089
q2 q3 4443 4897 4266 4266
q4 2066 2369 1381 1381
q5 4354 4280 4280 4280
q6 227 178 130 130
q7 2119 1893 1599 1599
q8 2455 2118 2081 2081
q9 7844 7681 7758 7681
q10 4524 4469 4050 4050
q11 576 410 367 367
q12 711 724 573 573
q13 3340 3620 3002 3002
q14 311 310 290 290
q15 q16 719 717 643 643
q17 1341 1311 1310 1310
q18 7920 7258 7111 7111
q19 1097 1089 1110 1089
q20 2200 2196 1921 1921
q21 5296 4590 4419 4419
q22 509 456 400 400
Total cold run time: 56223 ms
Total hot run time: 50682 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](json) Fix strict json validation [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3361472377
##########
regression-test/data/jsonb_p0/test_json_valid_strict.out:
##########
@@ -0,0 +1,4 @@
+-- This file is automatically generated. You should know what you did if you want to edit this
+-- !json_valid_strict --
+0 0 0 0 1
+
Review Comment:
`git diff --check` reports this as `new blank line at EOF`, so the standard whitespace check will fail even though the expected result itself is correct. Please regenerate or trim the `.out` file so it ends after the `0\t0\t0\t0\t1` result line.
```suggestion
```
--
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4467291965
/review
--
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg closed pull request #63309: [fix](json) Fix strict json validation
URL: https://github.com/apache/doris/pull/63309
--
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4495812938
/review
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4499982963
<details>
<summary>TPC-DS: <b>Total hot run time: 169887 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit a92ea1199e9f133540f9e9c62a72a61629dd687e, data reload: false
query5 4310 647 510 510
query6 322 225 205 205
query7 4212 556 312 312
query8 324 230 221 221
query9 8836 4041 4026 4026
query10 446 342 295 295
query11 5833 2368 2138 2138
query12 181 127 128 127
query13 1264 611 453 453
query14 5975 5410 5070 5070
query14_1 4380 4382 4355 4355
query15 209 210 186 186
query16 992 459 436 436
query17 1140 739 608 608
query18 2548 482 353 353
query19 214 199 166 166
query20 137 131 128 128
query21 212 140 122 122
query22 13675 13482 13328 13328
query23 17258 16294 16022 16022
query23_1 16160 16180 16144 16144
query24 7532 1753 1290 1290
query24_1 1280 1289 1285 1285
query25 551 459 423 423
query26 1299 316 176 176
query27 2719 563 348 348
query28 4453 1961 1950 1950
query29 994 612 490 490
query30 305 242 200 200
query31 1096 1059 932 932
query32 89 72 76 72
query33 532 351 307 307
query34 1166 1080 648 648
query35 752 782 665 665
query36 1325 1366 1196 1196
query37 158 109 86 86
query38 3161 3132 3040 3040
query39 918 904 908 904
query39_1 880 872 877 872
query40 230 146 124 124
query41 95 61 61 61
query42 108 108 107 107
query43 319 321 283 283
query44
query45 211 202 197 197
query46 1133 1181 741 741
query47 2332 2303 2231 2231
query48 400 408 292 292
query49 635 506 391 391
query50 1031 372 265 265
query51 4374 4396 4347 4347
query52 105 106 94 94
query53 247 279 200 200
query54 307 267 253 253
query55 93 89 83 83
query56 289 292 297 292
query57 1393 1400 1301 1301
query58 308 269 268 268
query59 1533 1579 1424 1424
query60 319 325 312 312
query61 150 157 150 150
query62 671 626 561 561
query63 245 203 206 203
query64 2410 803 638 638
query65
query66 1715 467 353 353
query67 29948 29945 29777 29777
query68
query69 500 339 303 303
query70 1054 991 930 930
query71 316 278 271 271
query72 2988 2712 2432 2432
query73 813 756 405 405
query74 5041 4857 4725 4725
query75 2643 2575 2241 2241
query76 2312 1105 762 762
query77 392 384 320 320
query78 11944 12128 11650 11650
query79 1322 974 721 721
query80 594 549 476 476
query81 451 282 237 237
query82 392 160 130 130
query83 366 279 254 254
query84 259 145 114 114
query85 982 594 519 519
query86 388 351 356 351
query87 3397 3380 3214 3214
query88 3509 2641 2637 2637
query89 438 384 347 347
query90 1949 179 181 179
query91 192 178 170 170
query92 80 78 73 73
query93 1466 1489 926 926
query94 523 358 308 308
query95 678 455 361 361
query96 1101 762 316 316
query97 2700 2692 2559 2559
query98 233 229 227 227
query99 1127 1107 986 986
Total cold run time: 251622 ms
Total hot run time: 169887 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4620276507
# BE UT Coverage Report
Increment line coverage `82.58% (128/155)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/8257d121b17ba41337a4cb57e6261f598e19ae46_8257d121b17ba41337a4cb57e6261f598e19ae46/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/8257d121b17ba41337a4cb57e6261f598e19ae46_8257d121b17ba41337a4cb57e6261f598e19ae46/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 53.94% (21060/39044) |
| Line Coverage | 37.60% (200239/532538) |
| Region Coverage | 33.68% (157047/466314) |
| Branch Coverage | 34.65% (68753/198406) |
--
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](json) Fix strict json validation and typed extraction [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4465209531
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: 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](json) Fix strict json validation [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3254107384
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -118,15 +105,7 @@ struct JsonbParser {
break;
}
case simdjson::ondemand::json_type::number: {
- simdjson::ondemand::number num;
- simdjson::error_code res = doc.get_number().get(num);
- if (!parse_number_success(res)) {
- return Status::InvalidArgument(fmt::format("simdjson get_number failed: {}",
- simdjson::error_message(res)));
- }
- // simdjson get_number() returns a number object, which can be
- RETURN_IF_ERROR(
- write_number(num, doc.get_number_type(), doc.raw_json_token(), writer));
+ RETURN_IF_ERROR(write_number_value(doc, writer));
break;
Review Comment:
This parse path still returns `OK` without verifying that the root value consumed the whole input. The new scalar checks cover cases like `nullx`/`100x`, but for object and array roots `parse(doc.get_value(), writer)` only consumes that value; without a final `doc.at_end()` (or equivalent) check, inputs such as `{}x` or `[]x` can be serialized as valid JSONB and `json_valid`/casts/ingestion will accept only the prefix. Please add a single-pass end-of-document validation after the switch and add tests for top-level object/array values followed by trailing garbage.
--
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](json) Fix strict json validation [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3252159303
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -93,6 +93,29 @@ struct JsonbParser {
simdjson::padded_string json_str {pch, len};
simdjson::ondemand::document doc = simdjson_parser.iterate(json_str);
+ auto is_json_whitespace = [](char c) {
+ return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+ };
+ const char* json_begin = json_str.data();
+ const char* json_end = json_str.data() + len;
+ while (json_begin < json_end && is_json_whitespace(*json_begin)) {
+ ++json_begin;
+ }
+ while (json_end > json_begin && is_json_whitespace(*(json_end - 1))) {
+ --json_end;
+ }
+
+ std::string_view raw_json;
+ simdjson::error_code raw_res = doc.raw_json().get(raw_json);
Review Comment:
This `raw_json()` call consumes the entire document, then `doc.rewind()` makes the code parse and serialize the same JSON again. `JsonbParser::parse` is shared by `json_valid`, `jsonb_parse`, casts to JSONB, and other ingestion paths, so large object/array inputs now pay an extra full simdjson traversal just to detect trailing content. The later code already consumes the root value and checks `doc.at_end()`, while top-level scalar getters use simdjson's root APIs that disallow trailing content and the new `is_null()` check covers the `not`/partial-null case. Please avoid the whole-document `raw_json()` pre-pass and enforce the remaining scalar/null validation in the existing parse pass so JSONB parsing stays single-pass.
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4469637486
# BE UT Coverage Report
Increment line coverage `80.00% (108/135)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/eaf2f141802181bd1c09fbe0dbe3b2a27414025f_eaf2f141802181bd1c09fbe0dbe3b2a27414025f/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/eaf2f141802181bd1c09fbe0dbe3b2a27414025f_eaf2f141802181bd1c09fbe0dbe3b2a27414025f/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 53.51% (20650/38593) |
| Line Coverage | 37.17% (195157/525093) |
| Region Coverage | 33.53% (152718/455436) |
| Branch Coverage | 34.57% (66590/192628) |
--
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](json) Fix strict json validation [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3272165503
##########
regression-test/suites/jsonb_p0/test_json_valid_strict.groovy:
##########
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_json_valid_strict") {
+ def jsonValid = sql """
Review Comment:
This deterministic regression case uses `sql` plus `assertEquals`, so it will not produce or validate a generated `.out` result. The regression-test standard in `AGENTS.md` says determined expected results should use `qt_sql`/`order_qt` style output instead of Groovy assertions, with result files generated by the test runner. Please convert this to a `qt_` query and add the generated `regression-test/data/jsonb_p0/test_json_valid_strict.out` (or move these checks into the existing generated-output json_p0 case).
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4504753701
<details>
<summary>TPC-DS: <b>Total hot run time: 169938 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 8257d121b17ba41337a4cb57e6261f598e19ae46, data reload: false
query5 4325 668 506 506
query6 361 219 203 203
query7 4229 544 305 305
query8 330 230 221 221
query9 8796 3970 3967 3967
query10 467 354 298 298
query11 5801 2401 2202 2202
query12 188 127 126 126
query13 1270 618 451 451
query14 6121 5374 5048 5048
query14_1 4349 4345 4337 4337
query15 215 211 189 189
query16 1039 471 464 464
query17 1169 747 617 617
query18 2752 504 361 361
query19 224 211 168 168
query20 141 136 133 133
query21 222 146 121 121
query22 13560 13539 13374 13374
query23 17219 16452 16080 16080
query23_1 16117 16193 16145 16145
query24 7471 1782 1330 1330
query24_1 1298 1316 1327 1316
query25 577 510 442 442
query26 1326 325 191 191
query27 2648 535 346 346
query28 4408 1964 1980 1964
query29 1011 653 517 517
query30 304 250 202 202
query31 1128 1068 945 945
query32 99 78 77 77
query33 556 364 341 341
query34 1143 1100 645 645
query35 774 776 678 678
query36 1311 1331 1197 1197
query37 146 97 89 89
query38 3181 3130 3040 3040
query39 922 936 896 896
query39_1 865 885 889 885
query40 233 145 128 128
query41 66 64 60 60
query42 106 108 110 108
query43 318 327 281 281
query44
query45 213 202 196 196
query46 1101 1183 751 751
query47 2350 2403 2213 2213
query48 390 421 284 284
query49 635 488 393 393
query50 1055 370 250 250
query51 4324 4312 4225 4225
query52 107 103 93 93
query53 260 288 204 204
query54 305 265 256 256
query55 93 99 92 92
query56 292 315 310 310
query57 1415 1410 1313 1313
query58 304 269 268 268
query59 1558 1646 1435 1435
query60 319 326 311 311
query61 160 179 158 158
query62 658 628 552 552
query63 238 199 204 199
query64 2400 793 616 616
query65
query66 1670 481 347 347
query67 29952 29891 29798 29798
query68
query69 467 337 308 308
query70 1039 994 954 954
query71 298 281 267 267
query72 2948 2687 2393 2393
query73 846 789 414 414
query74 5082 4897 4742 4742
query75 2675 2609 2257 2257
query76 2309 1120 766 766
query77 389 427 331 331
query78 12347 12194 11504 11504
query79 1483 1034 753 753
query80 656 554 456 456
query81 451 280 241 241
query82 1367 157 131 131
query83 357 277 254 254
query84 266 140 110 110
query85 919 553 449 449
query86 395 325 328 325
query87 3422 3352 3207 3207
query88 3504 2650 2679 2650
query89 443 380 334 334
query90 1863 188 175 175
query91 182 196 146 146
query92 80 76 70 70
query93 1525 1377 945 945
query94 544 354 302 302
query95 676 394 438 394
query96 1097 760 329 329
query97 2688 2700 2582 2582
query98 241 233 247 233
query99 1092 1107 998 998
Total cold run time: 252932 ms
Total hot run time: 169938 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4504884484
# BE UT Coverage Report
Increment line coverage `82.58% (128/155)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/8257d121b17ba41337a4cb57e6261f598e19ae46_8257d121b17ba41337a4cb57e6261f598e19ae46/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/8257d121b17ba41337a4cb57e6261f598e19ae46_8257d121b17ba41337a4cb57e6261f598e19ae46/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 53.57% (20704/38649) |
| Line Coverage | 37.16% (195993/527406) |
| Region Coverage | 33.50% (153685/458729) |
| Branch Coverage | 34.51% (66965/194033) |
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4630450526
# FE UT Coverage Report
Increment line coverage ` 100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2/fe_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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4629622700
/review
--
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4469509153
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4498590356
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4484124402
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4495914107
<details>
<summary>TPC-H: <b>Total hot run time: 31708 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 79bfbef96395e80cd3c5a7ebcaba6915ae800bbd, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17638 4016 3977 3977
q2 q3 10771 1427 824 824
q4 4708 479 348 348
q5 8109 2317 2171 2171
q6 367 179 139 139
q7 934 786 642 642
q8 9613 1723 1692 1692
q9 7035 4926 4962 4926
q10 6456 2075 1863 1863
q11 443 283 246 246
q12 640 440 310 310
q13 18142 3420 2791 2791
q14 266 254 236 236
q15 q16 806 777 711 711
q17 967 835 871 835
q18 6843 5736 5563 5563
q19 1196 1411 1199 1199
q20 567 425 284 284
q21 5857 2757 2629 2629
q22 459 380 322 322
Total cold run time: 101817 ms
Total hot run time: 31708 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4836 4554 4924 4554
q2 q3 4793 5285 4577 4577
q4 2249 2244 1427 1427
q5 4816 4745 4691 4691
q6 238 177 132 132
q7 1967 1706 1583 1583
q8 2250 2086 1915 1915
q9 7287 7207 7236 7207
q10 4504 4412 3976 3976
q11 562 402 376 376
q12 722 723 524 524
q13 3067 3404 2804 2804
q14 275 274 258 258
q15 q16 685 697 609 609
q17 1279 1255 1240 1240
q18 7380 6969 6654 6654
q19 1132 1129 1113 1113
q20 2252 2228 1943 1943
q21 5474 4864 4620 4620
q22 561 487 435 435
Total cold run time: 56329 ms
Total hot run time: 50638 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4500316187
# FE UT Coverage Report
Increment line coverage ` 100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_a92ea1199e9f133540f9e9c62a72a61629dd687e/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_a92ea1199e9f133540f9e9c62a72a61629dd687e/fe_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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4621537496
<details>
<summary>TPC-DS: <b>Total hot run time: 169197 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 8257d121b17ba41337a4cb57e6261f598e19ae46, data reload: false
query5 4320 626 478 478
query6 447 197 181 181
query7 4844 582 312 312
query8 373 223 222 222
query9 8775 4006 3991 3991
query10 469 317 274 274
query11 5958 2353 2262 2262
query12 163 101 103 101
query13 1285 610 431 431
query14 6409 5400 5047 5047
query14_1 4434 4387 4380 4380
query15 207 198 179 179
query16 1003 468 428 428
query17 932 689 568 568
query18 2428 481 344 344
query19 212 181 144 144
query20 115 110 103 103
query21 218 140 114 114
query22 13686 13686 13554 13554
query23 17313 16550 16154 16154
query23_1 16351 16327 16184 16184
query24 7558 1765 1299 1299
query24_1 1290 1323 1327 1323
query25 576 485 421 421
query26 1318 338 175 175
query27 2653 572 342 342
query28 4496 2007 2061 2007
query29 1129 642 507 507
query30 300 239 200 200
query31 1111 1076 954 954
query32 111 72 63 63
query33 536 318 269 269
query34 1206 1156 634 634
query35 758 831 669 669
query36 1392 1390 1247 1247
query37 155 105 91 91
query38 3195 3142 3079 3079
query39 935 939 895 895
query39_1 888 880 874 874
query40 226 121 103 103
query41 65 67 62 62
query42 98 92 92 92
query43 318 322 279 279
query44
query45 197 188 173 173
query46 1093 1178 735 735
query47 2324 2372 2204 2204
query48 422 426 297 297
query49 637 468 352 352
query50 976 350 264 264
query51 4314 4327 4258 4258
query52 91 88 78 78
query53 240 263 192 192
query54 278 219 204 204
query55 78 78 72 72
query56 250 214 221 214
query57 1436 1386 1332 1332
query58 245 214 230 214
query59 1591 1670 1464 1464
query60 279 254 238 238
query61 162 167 161 161
query62 694 678 589 589
query63 236 186 190 186
query64 2569 797 625 625
query65
query66 1815 472 349 349
query67 29848 29827 29547 29547
query68
query69 414 307 262 262
query70 978 966 982 966
query71 293 219 213 213
query72 3036 2713 2410 2410
query73 821 795 418 418
query74 5157 4972 4792 4792
query75 2684 2588 2234 2234
query76 2343 1149 803 803
query77 371 384 285 285
query78 12316 12398 11866 11866
query79 1247 1065 789 789
query80 530 484 405 405
query81 454 289 248 248
query82 234 162 122 122
query83 272 285 245 245
query84 305 147 117 117
query85 869 541 455 455
query86 339 306 282 282
query87 3361 3339 3190 3190
query88 3599 2742 2720 2720
query89 416 382 334 334
query90 2174 184 177 177
query91 173 169 141 141
query92 64 61 60 60
query93 1485 1534 921 921
query94 520 370 300 300
query95 668 493 349 349
query96 1084 789 356 356
query97 2723 2745 2586 2586
query98 212 209 206 206
query99 1167 1166 1027 1027
Total cold run time: 250506 ms
Total hot run time: 169197 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4631173581
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/40)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge_fe/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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4632237428
<details>
<summary>TPC-H: <b>Total hot run time: 29197 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 f6a987323ca9c51ca1901b7ae6f933763fb3faa2, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17752 4007 3931 3931
q2 q3 10745 1447 797 797
q4 4683 498 353 353
q5 7560 864 588 588
q6 178 169 134 134
q7 771 859 636 636
q8 9360 1657 1627 1627
q9 5938 4467 4487 4467
q10 6614 1817 1549 1549
q11 431 265 243 243
q12 626 426 303 303
q13 18118 3376 2755 2755
q14 267 270 242 242
q15 q16 809 785 699 699
q17 1006 862 966 862
q18 6933 5701 5531 5531
q19 1362 1345 1128 1128
q20 513 395 263 263
q21 6384 2762 2863 2762
q22 462 374 327 327
Total cold run time: 100512 ms
Total hot run time: 29197 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 5112 4995 4865 4865
q2 q3 4929 5286 4640 4640
q4 2165 2163 1398 1398
q5 4767 4854 4704 4704
q6 233 175 129 129
q7 1837 1831 1580 1580
q8 2412 2138 2115 2115
q9 7910 7563 7472 7472
q10 4728 4651 4191 4191
q11 533 386 352 352
q12 726 740 538 538
q13 2971 3365 2830 2830
q14 267 279 262 262
q15 q16 672 721 616 616
q17 1292 1266 1263 1263
q18 7335 6955 6726 6726
q19 1104 1074 1092 1074
q20 2213 2240 1950 1950
q21 5275 4613 4420 4420
q22 509 446 409 409
Total cold run time: 56990 ms
Total hot run time: 51534 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](json) Fix strict json validation [doris]
Posted by "HappenLee (via GitHub)" <gi...@apache.org>.
HappenLee commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3361065870
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -95,13 +84,20 @@ struct JsonbParser {
// simdjson process top level primitive types specially
// so some repeated code here
+ bool need_check_at_end = true;
switch (doc.type()) {
case simdjson::ondemand::json_type::object:
case simdjson::ondemand::json_type::array: {
RETURN_IF_ERROR(parse(doc.get_value(), writer));
break;
}
case simdjson::ondemand::json_type::null: {
+ bool is_null = false;
+ simdjson::error_code res = doc.is_null().get(is_null);
+ if (res != simdjson::SUCCESS || !is_null) {
+ return Status::InvalidArgument(fmt::format("simdjson get null failed: {}",
Review Comment:
如果is_null是false， 可能返回这个报错：· simdjson get null failed: SUCCESS 这个我觉得不太合理 .这个报错信息要改进一下
--
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4619702334
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4465683055
/review
--
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3274781463
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -281,54 +289,151 @@ struct JsonbParser {
case simdjson::ondemand::number_type::signed_integer:
case simdjson::ondemand::number_type::unsigned_integer: {
int128_t val = num.is_int64() ? (int128_t)num.get_int64() : (int128_t)num.get_uint64();
- bool success = false;
- if (val >= std::numeric_limits<int8_t>::min() &&
- val <= std::numeric_limits<int8_t>::max()) {
- success = writer.writeInt8((int8_t)val);
- } else if (val >= std::numeric_limits<int16_t>::min() &&
- val <= std::numeric_limits<int16_t>::max()) {
- success = writer.writeInt16((int16_t)val);
- } else if (val >= std::numeric_limits<int32_t>::min() &&
- val <= std::numeric_limits<int32_t>::max()) {
- success = writer.writeInt32((int32_t)val);
- } else if (val >= std::numeric_limits<int64_t>::min() &&
- val <= std::numeric_limits<int64_t>::max()) {
- success = writer.writeInt64((int64_t)val);
- } else { // INT128
- success = writer.writeInt128(val);
+ RETURN_IF_ERROR(write_int128(val, writer));
+ break;
+ }
+ case simdjson::ondemand::number_type::big_integer: {
+ RETURN_IF_ERROR(write_number_from_raw_json(raw_string, writer));
+ break;
+ }
+ }
+ return Status::OK();
+ }
+
+ static bool is_json_number_space(char c) {
+ return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+ }
+
+ static std::string_view trim_json_number(std::string_view raw_number) {
+ while (!raw_number.empty() && is_json_number_space(raw_number.front())) {
+ raw_number.remove_prefix(1);
+ }
+ while (!raw_number.empty() && is_json_number_space(raw_number.back())) {
+ raw_number.remove_suffix(1);
+ }
+ return raw_number;
+ }
+
+ static bool is_json_number_digit(char c) { return c >= '0' && c <= '9'; }
+
+ static Status validate_json_number(std::string_view raw_number, bool& is_integer) {
+ if (raw_number.empty()) {
+ return Status::InvalidArgument("empty number");
+ }
+
+ size_t pos = 0;
+ if (raw_number[pos] == '-') {
+ ++pos;
+ if (pos == raw_number.size()) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
+ }
- if (!success) {
- return Status::InvalidArgument("writeInt failed");
+ if (raw_number[pos] == '0') {
+ ++pos;
+ } else if (raw_number[pos] >= '1' && raw_number[pos] <= '9') {
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
}
- break;
+ } else {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
- case simdjson::ondemand::number_type::big_integer: {
- StringParser::ParseResult result;
- auto val = StringParser::string_to_int<int128_t>(raw_string.data(), raw_string.size(),
+
+ bool has_fraction = false;
+ if (pos < raw_number.size() && raw_number[pos] == '.') {
+ has_fraction = true;
+ ++pos;
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ bool has_exponent = false;
+ if (pos < raw_number.size() && (raw_number[pos] == 'e' || raw_number[pos] == 'E')) {
+ has_exponent = true;
+ ++pos;
+ if (pos < raw_number.size() && (raw_number[pos] == '+' || raw_number[pos] == '-')) {
+ ++pos;
+ }
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ if (pos != raw_number.size()) {
+ return Status::InvalidArgument("simdjson parse exception: trailing content");
+ }
+ is_integer = !has_fraction && !has_exponent;
+ return Status::OK();
+ }
+
+ static Status write_int128(int128_t val, JsonbWriter& writer) {
+ bool success = false;
+ if (val >= std::numeric_limits<int8_t>::min() &&
+ val <= std::numeric_limits<int8_t>::max()) {
+ success = writer.writeInt8((int8_t)val);
+ } else if (val >= std::numeric_limits<int16_t>::min() &&
+ val <= std::numeric_limits<int16_t>::max()) {
+ success = writer.writeInt16((int16_t)val);
+ } else if (val >= std::numeric_limits<int32_t>::min() &&
+ val <= std::numeric_limits<int32_t>::max()) {
+ success = writer.writeInt32((int32_t)val);
+ } else if (val >= std::numeric_limits<int64_t>::min() &&
+ val <= std::numeric_limits<int64_t>::max()) {
+ success = writer.writeInt64((int64_t)val);
+ } else { // INT128
+ success = writer.writeInt128(val);
+ }
+
+ if (!success) {
+ return Status::InvalidArgument("writeInt failed");
+ }
+ return Status::OK();
+ }
+
+ static Status write_number_from_raw_json(const char* pch, size_t len, JsonbWriter& writer) {
+ return write_number_from_raw_json(std::string_view(pch, len), writer);
+ }
+
+ // According to https://github.com/simdjson/simdjson/pull/2139
+ // For numbers larger than 64 bits, we can obtain the raw_json_token and parse it ourselves.
+ // This allows handling numbers larger than 64 bits, such as int128.
+ static Status write_number_from_raw_json(std::string_view raw_number, JsonbWriter& writer) {
+ raw_number = trim_json_number(raw_number);
+ bool is_integer = false;
+ RETURN_IF_ERROR(validate_json_number(raw_number, is_integer));
+
+ StringParser::ParseResult result;
+ if (is_integer) {
+ auto val = StringParser::string_to_int<int128_t>(raw_number.data(), raw_number.size(),
&result);
- if (result != StringParser::PARSE_SUCCESS) {
- // If the string exceeds the range of int128_t, it will attempt to convert it to double.
- // This may result in loss of precision, but for JSON, exchanging data as plain text between different systems may inherently cause precision loss.
- // try parse as double
- double double_val = StringParser::string_to_float<double>(
- raw_string.data(), raw_string.size(), &result);
- if (result != StringParser::PARSE_SUCCESS) {
- // if both parse failed, return error
- return Status::InvalidArgument("invalid number, raw string is: " +
- std::string(raw_string));
- }
- if (!writer.writeDouble(double_val)) {
- return Status::InvalidArgument("writeDouble failed");
- }
- } else {
- // as int128_t
- if (!writer.writeInt128(val)) {
- return Status::InvalidArgument("writeInt128 failed");
- }
+ if (result == StringParser::PARSE_SUCCESS) {
+ RETURN_IF_ERROR(write_int128(val, writer));
+ return Status::OK();
}
- break;
}
+
+ // If the string exceeds the range of int128_t, it will attempt to convert it to double.
+ // This may result in loss of precision, but for JSON, exchanging data as plain text
+ // between different systems may inherently cause precision loss.
+ double double_val = StringParser::string_to_float<double>(raw_number.data(),
+ raw_number.size(), &result);
+ if (result != StringParser::PARSE_SUCCESS) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
Review Comment:
`!std::isfinite(double_val)` will ensure the double value is finite.
--
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4494370693
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4504369005
# FE UT Coverage Report
Increment line coverage ` 100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46/fe_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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4485480754
/review
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4484434842
# FE UT Coverage Report
Increment line coverage ` 100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_02d620ae9a0606d7da74eb54d45a1697a6cfb72c/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_02d620ae9a0606d7da74eb54d45a1697a6cfb72c/fe_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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4484486370
<details>
<summary>TPC-DS: <b>Total hot run time: 170219 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 02d620ae9a0606d7da74eb54d45a1697a6cfb72c, data reload: false
query5 4339 686 536 536
query6 336 236 210 210
query7 4235 582 293 293
query8 328 235 224 224
query9 8812 4131 4081 4081
query10 465 352 307 307
query11 5793 2401 2267 2267
query12 187 133 133 133
query13 1288 612 402 402
query14 6023 5407 5101 5101
query14_1 4364 4409 4356 4356
query15 218 204 187 187
query16 1038 488 437 437
query17 1121 747 604 604
query18 2573 494 364 364
query19 227 218 171 171
query20 146 136 140 136
query21 215 151 128 128
query22 13673 13558 13384 13384
query23 17260 16472 16062 16062
query23_1 16164 16189 16319 16189
query24 7384 1826 1350 1350
query24_1 1332 1307 1331 1307
query25 610 525 455 455
query26 1328 329 177 177
query27 2698 587 339 339
query28 4446 2002 1979 1979
query29 1038 692 540 540
query30 310 247 203 203
query31 1120 1083 956 956
query32 86 84 78 78
query33 549 373 324 324
query34 1173 1164 677 677
query35 773 806 700 700
query36 1334 1340 1147 1147
query37 161 110 97 97
query38 3236 3139 3071 3071
query39 947 940 900 900
query39_1 874 864 871 864
query40 236 155 146 146
query41 72 71 70 70
query42 112 113 117 113
query43 349 356 309 309
query44
query45 213 207 204 204
query46 1111 1246 734 734
query47 2234 2254 2139 2139
query48 410 438 299 299
query49 653 517 424 424
query50 978 364 254 254
query51 4343 4307 4205 4205
query52 117 114 101 101
query53 265 300 218 218
query54 333 295 283 283
query55 101 97 93 93
query56 328 330 333 330
query57 1383 1364 1266 1266
query58 320 289 291 289
query59 1613 1689 1415 1415
query60 393 335 325 325
query61 161 163 163 163
query62 670 637 566 566
query63 251 216 209 209
query64 2423 806 626 626
query65
query66 1723 491 360 360
query67 30133 30029 30049 30029
query68
query69 462 349 309 309
query70 1055 1046 940 940
query71 317 292 283 283
query72 3171 2758 2480 2480
query73 853 799 437 437
query74 5079 4918 4745 4745
query75 2704 2614 2263 2263
query76 2300 1168 862 862
query77 410 428 339 339
query78 12082 12195 11632 11632
query79 1528 1057 772 772
query80 1322 558 455 455
query81 509 283 244 244
query82 1328 159 129 129
query83 355 279 252 252
query84 259 144 110 110
query85 947 567 442 442
query86 453 344 325 325
query87 3440 3390 3243 3243
query88 3616 2672 2689 2672
query89 462 383 344 344
query90 1918 193 186 186
query91 185 171 147 147
query92 84 78 76 76
query93 1568 1456 864 864
query94 719 362 308 308
query95 697 393 346 346
query96 1058 818 322 322
query97 2692 2703 2582 2582
query98 246 231 235 231
query99 1078 1080 972 972
Total cold run time: 255067 ms
Total hot run time: 170219 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](json) Fix strict json validation [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3273208407
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -281,54 +289,151 @@ struct JsonbParser {
case simdjson::ondemand::number_type::signed_integer:
case simdjson::ondemand::number_type::unsigned_integer: {
int128_t val = num.is_int64() ? (int128_t)num.get_int64() : (int128_t)num.get_uint64();
- bool success = false;
- if (val >= std::numeric_limits<int8_t>::min() &&
- val <= std::numeric_limits<int8_t>::max()) {
- success = writer.writeInt8((int8_t)val);
- } else if (val >= std::numeric_limits<int16_t>::min() &&
- val <= std::numeric_limits<int16_t>::max()) {
- success = writer.writeInt16((int16_t)val);
- } else if (val >= std::numeric_limits<int32_t>::min() &&
- val <= std::numeric_limits<int32_t>::max()) {
- success = writer.writeInt32((int32_t)val);
- } else if (val >= std::numeric_limits<int64_t>::min() &&
- val <= std::numeric_limits<int64_t>::max()) {
- success = writer.writeInt64((int64_t)val);
- } else { // INT128
- success = writer.writeInt128(val);
+ RETURN_IF_ERROR(write_int128(val, writer));
+ break;
+ }
+ case simdjson::ondemand::number_type::big_integer: {
+ RETURN_IF_ERROR(write_number_from_raw_json(raw_string, writer));
+ break;
+ }
+ }
+ return Status::OK();
+ }
+
+ static bool is_json_number_space(char c) {
+ return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+ }
+
+ static std::string_view trim_json_number(std::string_view raw_number) {
+ while (!raw_number.empty() && is_json_number_space(raw_number.front())) {
+ raw_number.remove_prefix(1);
+ }
+ while (!raw_number.empty() && is_json_number_space(raw_number.back())) {
+ raw_number.remove_suffix(1);
+ }
+ return raw_number;
+ }
+
+ static bool is_json_number_digit(char c) { return c >= '0' && c <= '9'; }
+
+ static Status validate_json_number(std::string_view raw_number, bool& is_integer) {
+ if (raw_number.empty()) {
+ return Status::InvalidArgument("empty number");
+ }
+
+ size_t pos = 0;
+ if (raw_number[pos] == '-') {
+ ++pos;
+ if (pos == raw_number.size()) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
+ }
- if (!success) {
- return Status::InvalidArgument("writeInt failed");
+ if (raw_number[pos] == '0') {
+ ++pos;
+ } else if (raw_number[pos] >= '1' && raw_number[pos] <= '9') {
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
}
- break;
+ } else {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
- case simdjson::ondemand::number_type::big_integer: {
- StringParser::ParseResult result;
- auto val = StringParser::string_to_int<int128_t>(raw_string.data(), raw_string.size(),
+
+ bool has_fraction = false;
+ if (pos < raw_number.size() && raw_number[pos] == '.') {
+ has_fraction = true;
+ ++pos;
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ bool has_exponent = false;
+ if (pos < raw_number.size() && (raw_number[pos] == 'e' || raw_number[pos] == 'E')) {
+ has_exponent = true;
+ ++pos;
+ if (pos < raw_number.size() && (raw_number[pos] == '+' || raw_number[pos] == '-')) {
+ ++pos;
+ }
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ if (pos != raw_number.size()) {
+ return Status::InvalidArgument("simdjson parse exception: trailing content");
+ }
+ is_integer = !has_fraction && !has_exponent;
+ return Status::OK();
+ }
+
+ static Status write_int128(int128_t val, JsonbWriter& writer) {
+ bool success = false;
+ if (val >= std::numeric_limits<int8_t>::min() &&
+ val <= std::numeric_limits<int8_t>::max()) {
+ success = writer.writeInt8((int8_t)val);
+ } else if (val >= std::numeric_limits<int16_t>::min() &&
+ val <= std::numeric_limits<int16_t>::max()) {
+ success = writer.writeInt16((int16_t)val);
+ } else if (val >= std::numeric_limits<int32_t>::min() &&
+ val <= std::numeric_limits<int32_t>::max()) {
+ success = writer.writeInt32((int32_t)val);
+ } else if (val >= std::numeric_limits<int64_t>::min() &&
+ val <= std::numeric_limits<int64_t>::max()) {
+ success = writer.writeInt64((int64_t)val);
+ } else { // INT128
+ success = writer.writeInt128(val);
+ }
+
+ if (!success) {
+ return Status::InvalidArgument("writeInt failed");
+ }
+ return Status::OK();
+ }
+
+ static Status write_number_from_raw_json(const char* pch, size_t len, JsonbWriter& writer) {
+ return write_number_from_raw_json(std::string_view(pch, len), writer);
+ }
+
+ // According to https://github.com/simdjson/simdjson/pull/2139
+ // For numbers larger than 64 bits, we can obtain the raw_json_token and parse it ourselves.
+ // This allows handling numbers larger than 64 bits, such as int128.
+ static Status write_number_from_raw_json(std::string_view raw_number, JsonbWriter& writer) {
+ raw_number = trim_json_number(raw_number);
+ bool is_integer = false;
+ RETURN_IF_ERROR(validate_json_number(raw_number, is_integer));
+
+ StringParser::ParseResult result;
+ if (is_integer) {
+ auto val = StringParser::string_to_int<int128_t>(raw_number.data(), raw_number.size(),
&result);
- if (result != StringParser::PARSE_SUCCESS) {
- // If the string exceeds the range of int128_t, it will attempt to convert it to double.
- // This may result in loss of precision, but for JSON, exchanging data as plain text between different systems may inherently cause precision loss.
- // try parse as double
- double double_val = StringParser::string_to_float<double>(
- raw_string.data(), raw_string.size(), &result);
- if (result != StringParser::PARSE_SUCCESS) {
- // if both parse failed, return error
- return Status::InvalidArgument("invalid number, raw string is: " +
- std::string(raw_string));
- }
- if (!writer.writeDouble(double_val)) {
- return Status::InvalidArgument("writeDouble failed");
- }
- } else {
- // as int128_t
- if (!writer.writeInt128(val)) {
- return Status::InvalidArgument("writeInt128 failed");
- }
+ if (result == StringParser::PARSE_SUCCESS) {
+ RETURN_IF_ERROR(write_int128(val, writer));
+ return Status::OK();
}
- break;
}
+
+ // If the string exceeds the range of int128_t, it will attempt to convert it to double.
+ // This may result in loss of precision, but for JSON, exchanging data as plain text
+ // between different systems may inherently cause precision loss.
+ double double_val = StringParser::string_to_float<double>(raw_number.data(),
+ raw_number.size(), &result);
+ if (result != StringParser::PARSE_SUCCESS) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
Review Comment:
This fallback only checks `PARSE_SUCCESS`, but `StringParser::string_to_float_internal()` marks success whenever `fast_float` consumes the whole token and does not inspect the overflow error code. For inputs such as `1e9999` (or an integer too large for `int128` and double), this can return `+/-inf`; `JsonbWriter::writeDouble()` stores it and `JsonbToJson` later formats it as `inf`, which is not valid JSON even though `json_valid`/casts accepted the input. Please reject non-finite fallback results, or use a parser path that propagates float overflow, and add a test for an overflowing exponent/huge integer.
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4495482840
# BE UT Coverage Report
Increment line coverage `70.73% (116/164)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/79bfbef96395e80cd3c5a7ebcaba6915ae800bbd_79bfbef96395e80cd3c5a7ebcaba6915ae800bbd/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/79bfbef96395e80cd3c5a7ebcaba6915ae800bbd_79bfbef96395e80cd3c5a7ebcaba6915ae800bbd/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 53.58% (20745/38718) |
| Line Coverage | 37.23% (196290/527210) |
| Region Coverage | 33.58% (153922/458425) |
| Branch Coverage | 34.61% (67079/193788) |
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4495792965
# BE Regression && UT Coverage Report
Increment line coverage `74.39% (122/164)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_79bfbef96395e80cd3c5a7ebcaba6915ae800bbd_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_79bfbef96395e80cd3c5a7ebcaba6915ae800bbd_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.62% (27917/37920) |
| Line Coverage | 57.58% (302790/525857) |
| Region Coverage | 54.64% (252902/462846) |
| Branch Coverage | 56.26% (109443/194515) |
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4501335741
# BE Regression && UT Coverage Report
Increment line coverage `79.76% (134/168)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_1b86d681ba30f35fbf1d31a4087fdeb73a6aba39_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_1b86d681ba30f35fbf1d31a4087fdeb73a6aba39_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.85% (27264/37944) |
| Line Coverage | 55.22% (290664/526418) |
| Region Coverage | 52.33% (242532/463446) |
| Branch Coverage | 53.62% (104503/194898) |
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4505399172
# BE Regression && UT Coverage Report
Increment line coverage `85.81% (133/155)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.63% (27874/37858) |
| Line Coverage | 57.55% (302770/526063) |
| Region Coverage | 54.77% (253669/463161) |
| Branch Coverage | 56.21% (109477/194760) |
--
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](json) Fix strict json validation [doris]
Posted by "HappenLee (via GitHub)" <gi...@apache.org>.
HappenLee commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3361046233
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -281,54 +293,151 @@ struct JsonbParser {
case simdjson::ondemand::number_type::signed_integer:
case simdjson::ondemand::number_type::unsigned_integer: {
int128_t val = num.is_int64() ? (int128_t)num.get_int64() : (int128_t)num.get_uint64();
- bool success = false;
- if (val >= std::numeric_limits<int8_t>::min() &&
- val <= std::numeric_limits<int8_t>::max()) {
- success = writer.writeInt8((int8_t)val);
- } else if (val >= std::numeric_limits<int16_t>::min() &&
- val <= std::numeric_limits<int16_t>::max()) {
- success = writer.writeInt16((int16_t)val);
- } else if (val >= std::numeric_limits<int32_t>::min() &&
- val <= std::numeric_limits<int32_t>::max()) {
- success = writer.writeInt32((int32_t)val);
- } else if (val >= std::numeric_limits<int64_t>::min() &&
- val <= std::numeric_limits<int64_t>::max()) {
- success = writer.writeInt64((int64_t)val);
- } else { // INT128
- success = writer.writeInt128(val);
+ RETURN_IF_ERROR(write_int128(val, writer));
+ break;
+ }
+ case simdjson::ondemand::number_type::big_integer: {
+ RETURN_IF_ERROR(write_number_from_raw_json(raw_string, writer));
+ break;
+ }
+ }
+ return Status::OK();
+ }
+
+ static bool is_json_number_space(char c) {
+ return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+ }
+
+ static std::string_view trim_json_number(std::string_view raw_number) {
+ while (!raw_number.empty() && is_json_number_space(raw_number.front())) {
+ raw_number.remove_prefix(1);
+ }
+ while (!raw_number.empty() && is_json_number_space(raw_number.back())) {
+ raw_number.remove_suffix(1);
+ }
+ return raw_number;
+ }
+
+ static bool is_json_number_digit(char c) { return c >= '0' && c <= '9'; }
+
+ static Status validate_json_number(std::string_view raw_number, bool& is_integer) {
+ if (raw_number.empty()) {
+ return Status::InvalidArgument("empty number");
+ }
+
+ size_t pos = 0;
+ if (raw_number[pos] == '-') {
+ ++pos;
+ if (pos == raw_number.size()) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
+ }
- if (!success) {
- return Status::InvalidArgument("writeInt failed");
+ if (raw_number[pos] == '0') {
+ ++pos;
+ } else if (raw_number[pos] >= '1' && raw_number[pos] <= '9') {
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
}
- break;
+ } else {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
- case simdjson::ondemand::number_type::big_integer: {
- StringParser::ParseResult result;
- auto val = StringParser::string_to_int<int128_t>(raw_string.data(), raw_string.size(),
+
+ bool has_fraction = false;
+ if (pos < raw_number.size() && raw_number[pos] == '.') {
+ has_fraction = true;
+ ++pos;
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ bool has_exponent = false;
+ if (pos < raw_number.size() && (raw_number[pos] == 'e' || raw_number[pos] == 'E')) {
+ has_exponent = true;
+ ++pos;
+ if (pos < raw_number.size() && (raw_number[pos] == '+' || raw_number[pos] == '-')) {
+ ++pos;
+ }
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ if (pos != raw_number.size()) {
+ return Status::InvalidArgument("simdjson parse exception: trailing content");
+ }
+ is_integer = !has_fraction && !has_exponent;
+ return Status::OK();
+ }
+
+ static Status write_int128(int128_t val, JsonbWriter& writer) {
+ bool success = false;
+ if (val >= std::numeric_limits<int8_t>::min() &&
+ val <= std::numeric_limits<int8_t>::max()) {
+ success = writer.writeInt8((int8_t)val);
+ } else if (val >= std::numeric_limits<int16_t>::min() &&
+ val <= std::numeric_limits<int16_t>::max()) {
+ success = writer.writeInt16((int16_t)val);
+ } else if (val >= std::numeric_limits<int32_t>::min() &&
+ val <= std::numeric_limits<int32_t>::max()) {
+ success = writer.writeInt32((int32_t)val);
+ } else if (val >= std::numeric_limits<int64_t>::min() &&
+ val <= std::numeric_limits<int64_t>::max()) {
+ success = writer.writeInt64((int64_t)val);
+ } else { // INT128
+ success = writer.writeInt128(val);
+ }
+
+ if (!success) {
+ return Status::InvalidArgument("writeInt failed");
+ }
+ return Status::OK();
+ }
+
+ static Status write_number_from_raw_json(const char* pch, size_t len, JsonbWriter& writer) {
Review Comment:
没必要实现这个重载，外面直接直接用户的时候调用stringview没什么代价
--
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](json) Fix strict json validation and typed extraction [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4465209604
/review
--
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](json) Fix strict json validation and typed extraction [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3251880082
##########
be/src/exprs/function/function_jsonb.cpp:
##########
@@ -1162,6 +1163,329 @@ struct JsonbExtractStringImpl {
} //function
};
+struct JsonbExtractStrictString {
+ static constexpr auto name = "jsonb_extract_string";
+ static constexpr auto alias = "json_extract_string";
+
+ using ReturnType = DataTypeString;
+ using ColumnType = ColumnString;
+
+private:
+ static ALWAYS_INLINE void inner_loop_impl(size_t i, ColumnString::Chars& res_data,
+ ColumnString::Offsets& res_offsets, NullMap& null_map,
+ const char* l_raw, size_t l_size, JsonbPath& path) {
+ const JsonbDocument* doc = nullptr;
+ auto st = JsonbDocument::checkAndCreateDocument(l_raw, l_size, &doc);
+ if (!st.ok() || !doc || !doc->getValue()) [[unlikely]] {
+ StringOP::push_null_string(i, res_data, res_offsets, null_map);
+ return;
+ }
+
+ auto find_result = doc->getValue()->findValue(path);
+ const auto* value = find_result.value;
+ if (UNLIKELY(!value) || !value->isString()) {
+ StringOP::push_null_string(i, res_data, res_offsets, null_map);
+ return;
+ }
+
+ const auto* str_value = value->unpack<JsonbStringVal>();
+ StringOP::push_value_string(std::string_view(str_value->getBlob(), str_value->length()), i,
+ res_data, res_offsets);
+ }
+
+public:
+ static Status vector_vector_v2(FunctionContext* context, const ColumnString::Chars& ldata,
+ const ColumnString::Offsets& loffsets, const NullMap* l_null_map,
+ const bool& json_data_const,
+ const std::vector<const ColumnString*>& rdata_columns,
+ const std::vector<const NullMap*>& r_null_maps,
+ const std::vector<bool>& path_const,
+ ColumnString::Chars& res_data,
+ ColumnString::Offsets& res_offsets, NullMap& null_map) {
+ DCHECK_EQ(rdata_columns.size(), 1);
+ const size_t input_rows_count = null_map.size();
+ res_offsets.resize(input_rows_count);
+
+ JsonbPath json_path;
+ auto parse_json_path = [&](size_t i) -> Status {
+ const auto index = index_check_const(i, path_const[0]);
+
+ const ColumnString* path_col = rdata_columns[0];
+ const ColumnString::Chars& rdata = path_col->get_chars();
+ const ColumnString::Offsets& roffsets = path_col->get_offsets();
+ size_t r_off = roffsets[index - 1];
+ size_t r_size = roffsets[index] - r_off;
+ const char* r_raw = reinterpret_cast<const char*>(&rdata[r_off]);
+
+ JsonbPath path;
+ if (!path.seek(r_raw, r_size)) {
+ return Status::InvalidArgument("Json path error: Invalid Json Path for value: {}",
+ std::string_view(r_raw, r_size));
+ }
+
+ json_path = std::move(path);
+ return Status::OK();
+ };
+
+ if (path_const[0]) {
+ if (r_null_maps[0] && (*r_null_maps[0])[0]) {
+ for (size_t i = 0; i < input_rows_count; ++i) {
+ StringOP::push_null_string(i, res_data, res_offsets, null_map);
+ }
+ return Status::OK();
+ }
+ RETURN_IF_ERROR(parse_json_path(0));
+ }
+
+ res_data.reserve(ldata.size());
+ for (size_t i = 0; i < input_rows_count; ++i) {
+ if (null_map[i]) {
+ continue;
+ }
+
+ const auto data_index = index_check_const(i, json_data_const);
+ if (l_null_map && (*l_null_map)[data_index]) {
+ StringOP::push_null_string(i, res_data, res_offsets, null_map);
+ continue;
+ }
+
+ const auto path_index = index_check_const(i, path_const[0]);
+ if (r_null_maps[0] && (*r_null_maps[0])[path_index]) {
+ StringOP::push_null_string(i, res_data, res_offsets, null_map);
+ continue;
+ }
+
+ if (!path_const[0]) {
+ RETURN_IF_ERROR(parse_json_path(i));
+ }
+
+ size_t l_off = loffsets[data_index - 1];
+ size_t l_size = loffsets[data_index] - l_off;
+ const char* l_raw = reinterpret_cast<const char*>(&ldata[l_off]);
+ inner_loop_impl(i, res_data, res_offsets, null_map, l_raw, l_size, json_path);
+ }
+ return Status::OK();
+ }
+};
+
+template <typename Derived, typename ResultDataType, typename ResultColumnType>
+struct JsonbExtractTypedNumberImpl {
+ using ReturnType = ResultDataType;
+ using ColumnType = ResultColumnType;
+ using Container = typename ColumnType::Container;
+
+private:
+ static ALWAYS_INLINE void insert_null(size_t i, Container& res, NullMap& null_map) {
+ res[i] = 0;
+ null_map[i] = 1;
+ }
+
+ static ALWAYS_INLINE void inner_loop_impl(size_t i, Container& res, NullMap& null_map,
+ const char* l_raw_str, size_t l_str_size,
+ JsonbPath& path) {
+ if (null_map[i]) {
+ res[i] = 0;
+ return;
+ }
+
+ const JsonbDocument* doc = nullptr;
+ auto st = JsonbDocument::checkAndCreateDocument(l_raw_str, l_str_size, &doc);
+ if (!st.ok() || !doc || !doc->getValue()) [[unlikely]] {
+ insert_null(i, res, null_map);
+ return;
+ }
+
+ auto find_result = doc->getValue()->findValue(path);
+ const auto* value = find_result.value;
+ if (UNLIKELY(!value) || !Derived::extract_value(value, res[i])) {
+ insert_null(i, res, null_map);
+ }
+ }
+
+public:
+ static Status vector_vector(FunctionContext* context, const ColumnString::Chars& ldata,
+ const ColumnString::Offsets& loffsets, const NullMap* l_null_map,
+ const ColumnString::Chars& rdata,
+ const ColumnString::Offsets& roffsets, const NullMap* r_null_map,
+ Container& res, NullMap& null_map) {
+ size_t size = loffsets.size();
+ res.resize(size);
+
+ for (size_t i = 0; i < loffsets.size(); i++) {
+ if ((l_null_map && (*l_null_map)[i]) || (r_null_map && (*r_null_map)[i])) {
+ insert_null(i, res, null_map);
+ continue;
+ }
+
+ const char* l_raw_str = reinterpret_cast<const char*>(&ldata[loffsets[i - 1]]);
+ int l_str_size = loffsets[i] - loffsets[i - 1];
+
+ const char* r_raw_str = reinterpret_cast<const char*>(&rdata[roffsets[i - 1]]);
+ int r_str_size = roffsets[i] - roffsets[i - 1];
+
+ JsonbPath path;
+ if (!path.seek(r_raw_str, r_str_size)) {
+ return Status::InvalidArgument(
+ "Json path error: Invalid Json Path for value: {} at row: {}",
+ std::string_view(r_raw_str, r_str_size), i);
+ }
+
+ inner_loop_impl(i, res, null_map, l_raw_str, l_str_size, path);
+ }
+ return Status::OK();
+ }
+
+ static Status scalar_vector(FunctionContext* context, const StringRef& ldata,
+ const ColumnString::Chars& rdata,
+ const ColumnString::Offsets& roffsets, const NullMap* r_null_map,
+ Container& res, NullMap& null_map) {
+ size_t size = roffsets.size();
+ res.resize(size);
Review Comment:
This sizes the result from the unwrapped path column, which is only one row when both the JSON argument and path argument are `ColumnConst`. `FunctionJsonbExtract::execute_impl` checks `jsonb_data_const` first and calls this `scalar_vector` branch even when `path_const[0]` is also true, so a query that evaluates `jsonb_extract_int(jsonb_parse('{"a":1}'), '$.a')` over a multi-row block can build a nested result column of size 1 and pair it with a null map sized to `input_rows_count`. That can either trip column-size invariants or return only the first row. Please handle the const/const case explicitly, or pass `input_rows_count` into this branch and fill that many result rows.
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4469538670
<details>
<summary>TPC-H: <b>Total hot run time: 31094 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 eaf2f141802181bd1c09fbe0dbe3b2a27414025f, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17731 3884 3922 3884
q2 q3 10799 1375 813 813
q4 4681 468 351 351
q5 7566 2234 2124 2124
q6 230 176 137 137
q7 927 782 633 633
q8 9340 1674 1698 1674
q9 5146 4931 4917 4917
q10 6402 2059 1778 1778
q11 433 264 241 241
q12 639 421 293 293
q13 18142 3386 2786 2786
q14 260 256 231 231
q15 q16 815 768 706 706
q17 945 977 971 971
q18 6724 5860 5441 5441
q19 1310 1310 1134 1134
q20 504 413 265 265
q21 5810 2522 2409 2409
q22 423 361 306 306
Total cold run time: 98827 ms
Total hot run time: 31094 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4186 4107 4116 4107
q2 q3 4507 4925 4320 4320
q4 2115 2199 1383 1383
q5 4366 4290 4299 4290
q6 237 171 134 134
q7 1748 2127 1848 1848
q8 2525 2131 2122 2122
q9 8032 8018 7785 7785
q10 4493 4482 4086 4086
q11 566 423 373 373
q12 755 772 512 512
q13 3342 3582 2938 2938
q14 298 300 258 258
q15 q16 700 758 650 650
q17 1326 1308 1312 1308
q18 8025 7327 7291 7291
q19 1202 1134 1174 1134
q20 2223 2209 1920 1920
q21 5276 4617 4499 4499
q22 514 472 406 406
Total cold run time: 56436 ms
Total hot run time: 51364 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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4629707679
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on code in PR #63309:
URL: https://github.com/apache/doris/pull/63309#discussion_r3361390950
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -281,54 +293,151 @@ struct JsonbParser {
case simdjson::ondemand::number_type::signed_integer:
case simdjson::ondemand::number_type::unsigned_integer: {
int128_t val = num.is_int64() ? (int128_t)num.get_int64() : (int128_t)num.get_uint64();
- bool success = false;
- if (val >= std::numeric_limits<int8_t>::min() &&
- val <= std::numeric_limits<int8_t>::max()) {
- success = writer.writeInt8((int8_t)val);
- } else if (val >= std::numeric_limits<int16_t>::min() &&
- val <= std::numeric_limits<int16_t>::max()) {
- success = writer.writeInt16((int16_t)val);
- } else if (val >= std::numeric_limits<int32_t>::min() &&
- val <= std::numeric_limits<int32_t>::max()) {
- success = writer.writeInt32((int32_t)val);
- } else if (val >= std::numeric_limits<int64_t>::min() &&
- val <= std::numeric_limits<int64_t>::max()) {
- success = writer.writeInt64((int64_t)val);
- } else { // INT128
- success = writer.writeInt128(val);
+ RETURN_IF_ERROR(write_int128(val, writer));
+ break;
+ }
+ case simdjson::ondemand::number_type::big_integer: {
+ RETURN_IF_ERROR(write_number_from_raw_json(raw_string, writer));
+ break;
+ }
+ }
+ return Status::OK();
+ }
+
+ static bool is_json_number_space(char c) {
+ return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+ }
+
+ static std::string_view trim_json_number(std::string_view raw_number) {
+ while (!raw_number.empty() && is_json_number_space(raw_number.front())) {
+ raw_number.remove_prefix(1);
+ }
+ while (!raw_number.empty() && is_json_number_space(raw_number.back())) {
+ raw_number.remove_suffix(1);
+ }
+ return raw_number;
+ }
+
+ static bool is_json_number_digit(char c) { return c >= '0' && c <= '9'; }
+
+ static Status validate_json_number(std::string_view raw_number, bool& is_integer) {
+ if (raw_number.empty()) {
+ return Status::InvalidArgument("empty number");
+ }
+
+ size_t pos = 0;
+ if (raw_number[pos] == '-') {
+ ++pos;
+ if (pos == raw_number.size()) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
+ }
- if (!success) {
- return Status::InvalidArgument("writeInt failed");
+ if (raw_number[pos] == '0') {
+ ++pos;
+ } else if (raw_number[pos] >= '1' && raw_number[pos] <= '9') {
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
}
- break;
+ } else {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
}
- case simdjson::ondemand::number_type::big_integer: {
- StringParser::ParseResult result;
- auto val = StringParser::string_to_int<int128_t>(raw_string.data(), raw_string.size(),
+
+ bool has_fraction = false;
+ if (pos < raw_number.size() && raw_number[pos] == '.') {
+ has_fraction = true;
+ ++pos;
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ bool has_exponent = false;
+ if (pos < raw_number.size() && (raw_number[pos] == 'e' || raw_number[pos] == 'E')) {
+ has_exponent = true;
+ ++pos;
+ if (pos < raw_number.size() && (raw_number[pos] == '+' || raw_number[pos] == '-')) {
+ ++pos;
+ }
+ if (pos == raw_number.size() || !is_json_number_digit(raw_number[pos])) {
+ return Status::InvalidArgument("invalid number, raw string is: " +
+ std::string(raw_number));
+ }
+ while (pos < raw_number.size() && is_json_number_digit(raw_number[pos])) {
+ ++pos;
+ }
+ }
+
+ if (pos != raw_number.size()) {
+ return Status::InvalidArgument("simdjson parse exception: trailing content");
+ }
+ is_integer = !has_fraction && !has_exponent;
+ return Status::OK();
+ }
+
+ static Status write_int128(int128_t val, JsonbWriter& writer) {
+ bool success = false;
+ if (val >= std::numeric_limits<int8_t>::min() &&
+ val <= std::numeric_limits<int8_t>::max()) {
+ success = writer.writeInt8((int8_t)val);
+ } else if (val >= std::numeric_limits<int16_t>::min() &&
+ val <= std::numeric_limits<int16_t>::max()) {
+ success = writer.writeInt16((int16_t)val);
+ } else if (val >= std::numeric_limits<int32_t>::min() &&
+ val <= std::numeric_limits<int32_t>::max()) {
+ success = writer.writeInt32((int32_t)val);
+ } else if (val >= std::numeric_limits<int64_t>::min() &&
+ val <= std::numeric_limits<int64_t>::max()) {
+ success = writer.writeInt64((int64_t)val);
+ } else { // INT128
+ success = writer.writeInt128(val);
+ }
+
+ if (!success) {
+ return Status::InvalidArgument("writeInt failed");
+ }
+ return Status::OK();
+ }
+
+ static Status write_number_from_raw_json(const char* pch, size_t len, JsonbWriter& writer) {
Review Comment:
已按建议删除这个 char*/len 重载，调用处直接构造 std::string_view(pch, len) 后调用 string_view 版本。
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -95,13 +84,20 @@ struct JsonbParser {
// simdjson process top level primitive types specially
// so some repeated code here
+ bool need_check_at_end = true;
switch (doc.type()) {
case simdjson::ondemand::json_type::object:
case simdjson::ondemand::json_type::array: {
RETURN_IF_ERROR(parse(doc.get_value(), writer));
break;
}
case simdjson::ondemand::json_type::null: {
+ bool is_null = false;
+ simdjson::error_code res = doc.is_null().get(is_null);
+ if (res != simdjson::SUCCESS || !is_null) {
+ return Status::InvalidArgument(fmt::format("simdjson get null failed: {}",
Review Comment:
已拆开 simdjson 调用失败和 !is_null 两种情况；当类型判断为 null 但 is_null 为 false 时，现在返回 invalid JSON null literal，避免出现 simdjson get null failed: SUCCESS 这种误导信息。
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4631081172
# BE Regression && UT Coverage Report
Increment line coverage `85.16% (132/155)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.81% (28242/38261) |
| Line Coverage | 57.85% (307393/531343) |
| Region Coverage | 54.50% (256701/471023) |
| Branch Coverage | 56.02% (111645/199297) |
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4633823319
# BE Regression && UT Coverage Report
Increment line coverage `85.16% (132/155)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_f6a987323ca9c51ca1901b7ae6f933763fb3faa2_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.81% (28242/38261) |
| Line Coverage | 57.84% (307333/531343) |
| Region Coverage | 54.50% (256713/471023) |
| Branch Coverage | 56.00% (111613/199297) |
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4620880939
# BE Regression && UT Coverage Report
Increment line coverage `85.81% (133/155)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_8257d121b17ba41337a4cb57e6261f598e19ae46_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.89% (27507/38260) |
| Line Coverage | 55.43% (294500/531266) |
| Region Coverage | 52.27% (246163/470936) |
| Branch Coverage | 53.35% (106294/199255) |
--
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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4621454183
<details>
<summary>TPC-H: <b>Total hot run time: 29135 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 8257d121b17ba41337a4cb57e6261f598e19ae46, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17714 4158 4069 4069
q2 q3 10864 1428 876 876
q4 4695 480 358 358
q5 7606 905 599 599
q6 185 171 136 136
q7 781 862 630 630
q8 9427 1623 1670 1623
q9 5865 4534 4537 4534
q10 6781 1844 1526 1526
q11 431 264 247 247
q12 633 437 295 295
q13 18134 3328 2752 2752
q14 263 259 236 236
q15 q16 827 794 710 710
q17 1010 1018 910 910
q18 6900 5676 5541 5541
q19 1303 1405 943 943
q20 543 401 259 259
q21 6339 2869 2567 2567
q22 471 375 324 324
Total cold run time: 100772 ms
Total hot run time: 29135 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 5088 4854 4673 4673
q2 q3 4870 5306 4728 4728
q4 2145 2255 1436 1436
q5 4833 4828 4727 4727
q6 234 174 130 130
q7 1876 1708 1738 1708
q8 2442 2123 2085 2085
q9 7988 7684 7423 7423
q10 4763 4708 4240 4240
q11 543 382 357 357
q12 742 754 530 530
q13 2974 3388 2822 2822
q14 281 294 264 264
q15 q16 685 710 614 614
q17 1294 1250 1261 1250
q18 7224 6870 6929 6870
q19 1106 1107 1138 1107
q20 2212 2213 1957 1957
q21 5312 4640 4418 4418
q22 529 476 435 435
Total cold run time: 57141 ms
Total hot run time: 51774 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4484422051
<details>
<summary>TPC-H: <b>Total hot run time: 31868 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 02d620ae9a0606d7da74eb54d45a1697a6cfb72c, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17634 4044 4049 4044
q2 q3 10828 1455 828 828
q4 4716 479 348 348
q5 8403 2319 2117 2117
q6 389 187 146 146
q7 1001 754 621 621
q8 9581 1789 1740 1740
q9 7059 4938 4949 4938
q10 6493 2228 1841 1841
q11 438 276 249 249
q12 684 439 304 304
q13 18144 3498 2831 2831
q14 271 257 241 241
q15 q16 819 783 721 721
q17 1017 973 1014 973
q18 6764 5699 5660 5660
q19 1307 1318 1118 1118
q20 514 415 273 273
q21 6148 2842 2557 2557
q22 469 378 318 318
Total cold run time: 102679 ms
Total hot run time: 31868 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4951 5059 4897 4897
q2 q3 4876 5278 4600 4600
q4 2214 2310 1462 1462
q5 4863 4750 4677 4677
q6 249 181 142 142
q7 1947 1722 1528 1528
q8 2324 2111 1993 1993
q9 7304 7312 7380 7312
q10 4484 4408 4033 4033
q11 552 409 364 364
q12 726 745 520 520
q13 3037 3493 2820 2820
q14 285 281 260 260
q15 q16 692 706 616 616
q17 1299 1253 1278 1253
q18 7348 6984 6931 6931
q19 1144 1101 1130 1101
q20 2217 2226 1944 1944
q21 5520 4817 4747 4747
q22 548 475 435 435
Total cold run time: 56580 ms
Total hot run time: 51635 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](json) Fix strict json validation [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4501437617
# FE Regression Coverage Report
Increment line coverage ` 100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63309_1b86d681ba30f35fbf1d31a4087fdeb73a6aba39_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63309_1b86d681ba30f35fbf1d31a4087fdeb73a6aba39_merge_fe/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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4499458767
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](json) Fix strict json validation [doris]
Posted by "mrhhsg (via GitHub)" <gi...@apache.org>.
mrhhsg commented on PR #63309:
URL: https://github.com/apache/doris/pull/63309#issuecomment-4496587583
/review
--
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