You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "starocean999 (via GitHub)" <gi...@apache.org> on 2026/10/20 01:28:28 UTC
[PR] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
starocean999 opened a new pull request, #69305:
URL: https://github.com/apache/doris/pull/69305
Problem Summary:
Audit events for `INSERT INTO ... SELECT` always had an empty `sql_digest`. Digest generation only ran inside the `is_query` branch of `AuditLogHelper`, while INSERT is a Nereids `Command` and therefore has `is_query=false`. `InsertIntoTableCommand.toDigest()` already exists, but it represents the whole INSERT statement and includes the write sink.
This change:
- Adds `AuditDigestProvider` for non-query commands that need an audit fingerprint.
- Extracts the SELECT-only digest from the original parser plan in `InsertUtils.getSelectDigestForAudit`: the write sink is excluded, a CTE is preserved, and literals stay normalized. Write-time casts and default columns are not included.
- Decouples audit digest generation from the query metrics branch in `AuditLogHelper`. `updateMetricsImpl` remains query-only, and `is_query` is unchanged.
- Keeps `sql_digest` empty for `INSERT ... VALUES` because there is no SELECT semantics to fingerprint.
As a result, `INSERT INTO t SELECT ...` gets the same digest as the equivalent standalone SELECT. This PR intentionally scopes to `INSERT INTO ... SELECT`; `INSERT OVERWRITE ... SELECT`, `INSERT INTO TVF ... SELECT`, and the legacy planner are not covered.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6286587870
# FE Regression Coverage Report
Increment line coverage ` 8.93% (20/224)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69305_18a368d0494c850fb01eba1ff0269eaf09f70f05_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69305_18a368d0494c850fb01eba1ff0269eaf09f70f05_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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6285958929
<details>
<summary>ClickBench: <b>Total hot run time: 24.31 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 18a368d0494c850fb01eba1ff0269eaf09f70f05, data reload: false
query1 0.01 0.01 0.01
query2 0.09 0.05 0.05
query3 0.26 0.09 0.08
query4 1.61 0.10 0.11
query5 0.23 0.24 0.24
query6 1.18 0.67 0.68
query7 0.04 0.01 0.00
query8 0.06 0.03 0.04
query9 0.40 0.35 0.34
query10 0.56 0.58 0.57
query11 0.21 0.14 0.15
query12 0.18 0.14 0.14
query13 0.50 0.49 0.49
query14 1.02 1.00 1.03
query15 0.64 0.60 0.60
query16 0.35 0.34 0.35
query17 1.16 1.12 1.15
query18 0.22 0.18 0.20
query19 2.00 1.99 1.98
query20 0.02 0.01 0.02
query21 15.52 0.21 0.13
query22 4.86 0.06 0.05
query23 16.27 0.34 0.13
query24 3.21 0.51 0.40
query25 0.11 0.11 0.06
query26 0.79 0.21 0.15
query27 0.07 0.04 0.06
query28 3.72 0.78 0.37
query29 12.47 4.18 3.31
query30 0.28 0.13 0.09
query31 2.77 0.55 0.32
query32 3.22 0.58 0.51
query33 3.20 3.12 3.23
query34 15.41 4.00 3.43
query35 3.35 3.34 3.32
query36 0.56 0.45 0.44
query37 0.11 0.07 0.06
query38 0.06 0.04 0.04
query39 0.04 0.03 0.03
query40 0.17 0.16 0.14
query41 0.09 0.03 0.03
query42 0.04 0.02 0.02
query43 0.04 0.03 0.03
Total cold run time: 97.1 s
Total hot run time: 24.31 s
```
</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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6281177369
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://doris.apache.org/community/how-to-contribute/review-and-ci).
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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "starocean999 (via GitHub)" <gi...@apache.org>.
starocean999 commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6283709712
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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "starocean999 (via GitHub)" <gi...@apache.org>.
starocean999 commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6281179227
/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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69305:
URL: https://github.com/apache/doris/pull/69305#discussion_r4320614397
##########
fe/fe-core/src/main/java/org/apache/doris/qe/AuditLogHelper.java:
##########
@@ -363,19 +364,22 @@ private static void logAuditLogImpl(ConnectContext ctx, String origStmt, Stateme
if (ctx.getState().isQuery()) {
updateMetricsImpl(ctx);
- if (MetricRepo.isInit && !ctx.getState().isInternal()) {
- if ((ctx.getState().getStateType() == MysqlStateType.OK
- || ctx.getState().getStateType() == MysqlStateType.EOF)
- && elapseMs > Config.sql_digest_generation_threshold_ms) {
- String sqlDigest = DigestUtils.md5Hex(((Queriable) parsedStmt).toDigest());
- auditEventBuilder.setSqlDigest(sqlDigest);
- }
- }
auditEventBuilder.setScanBytesFromLocalStorage(
statistics == null ? 0 : statistics.getScanBytesFromLocalStorage())
.setScanBytesFromRemoteStorage(
statistics == null ? 0 : statistics.getScanBytesFromRemoteStorage());
}
+ // DML statements such as INSERT INTO ... SELECT are not classified as queries, but the audit
+ // log still needs their sql_digest. Keep this independent from updateMetricsImpl so that
+ // is_query and the query metrics stay unchanged.
+ if ((!ctx.getState().isQuery() || MetricRepo.isInit)
+ && !ctx.getState().isInternal()
+ && (ctx.getState().getStateType() == MysqlStateType.OK
Review Comment:
[P2] Use the forwarded status when gating INSERT digests
On a MySQL follower, `INSERT INTO dst SELECT * FROM missing_src` is forwarded before source validation. If the master rejects it and the digest threshold is exceeded, FEOpExecutor retains that ERR in its result while the local QueryState remains OK. This gate therefore generates the new INSERT digest; only later does this method replace the audit event's state/error with the proxy result. The resulting ERR event has a nonempty sql_digest, whereas the same failed INSERT executed on the master is excluded by this success gate. Resolve the effective proxy/local status before deciding digest eligibility, and cover a forwarded failed INSERT in the audit tests.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -664,6 +664,28 @@ public static TableIf getTargetTable(Plan plan, ConnectContext ctx) {
return RelationUtil.getTable(tableQualifier, ctx.getEnv(), Optional.empty());
}
+ /**
+ * Build the SELECT-only digest recorded by the audit log for {@code INSERT ... SELECT}.
+ * <p>
+ * The digest must come from the original parse tree: {@code originLogicalQuery} is the
+ * un-analyzed sink built by the parser, so its child has none of the casts, default columns or
+ * other rewrites introduced while planning the write. When the statement carries a WITH
+ * clause, {@code cte} wraps that same child and its digest already contains the WITH list.
+ *
+ * @return the digest, or empty when the source is a VALUES inline table rather than a query
+ */
+ public static Optional<String> getSelectDigestForAudit(LogicalPlan originLogicalQuery,
+ Optional<LogicalPlan> cte) {
+ if (cte.isPresent()) {
Review Comment:
[P2] Exclude WITH-wrapped VALUES before generating a digest
`WITH c AS (SELECT * FROM src) INSERT INTO dst VALUES (4)` is supported (see the existing `insert_cte.groovy:49` regression). Its retained source is:
```text
originLogicalQuery: Sink -> InlineTable
cte: LogicalCTE(c) -> InlineTable
```
This early return fingerprints `WITH ... VALUES ?` before reaching the InlineTable exclusion, so an eligible successful VALUES insert gets a nonempty sql_digest despite the promised VALUES-empty behavior. A query-local WITH wrapper also bypasses the bare InlineTable check below. Classify the actual source through its WITH wrapper before returning a digest, and add negative coverage for these VALUES forms.
--
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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "starocean999 (via GitHub)" <gi...@apache.org>.
starocean999 commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6283322314
/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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6285717129
<details>
<summary>TPC-H: <b>Total hot run time: 28566 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 18a368d0494c850fb01eba1ff0269eaf09f70f05, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17586 5118 5039 5039
q2 2256 312 282 282
q3 10090 1419 815 815
q4 4682 457 343 343
q5 7463 806 523 523
q6 185 184 156 156
q7 766 794 598 598
q8 9237 1335 1021 1021
q9 5580 4168 4195 4168
q10 6819 1290 1042 1042
q11 438 247 229 229
q12 651 399 276 276
q13 18156 6254 3032 3032
q14 282 274 242 242
q15 q16 743 716 663 663
q17 1214 747 571 571
q18 7105 6201 6141 6141
q19 1261 970 598 598
q20 388 336 227 227
q21 5552 2325 2411 2325
q22 401 320 275 275
Total cold run time: 100855 ms
Total hot run time: 28566 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 5971 5894 6070 5894
q2 740 602 572 572
q3 5690 5845 5291 5291
q4 1970 2132 1584 1584
q5 5771 5607 5676 5607
q6 234 189 138 138
q7 2205 2033 1837 1837
q8 2994 2622 2573 2573
q9 7895 7913 7809 7809
q10 3611 3594 3176 3176
q11 575 412 375 375
q12 679 654 494 494
q13 3251 3606 3015 3015
q14 285 287 264 264
q15 q16 680 706 613 613
q17 7699 7004 6869 6869
q18 12951 12204 12854 12204
q19 884 771 771 771
q20 2144 2165 1910 1910
q21 5506 4532 4703 4532
q22 498 444 395 395
Total cold run time: 72233 ms
Total hot run time: 65923 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] [feature](fe) Record SELECT-only sql_digest for INSERT INTO SELECT [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69305:
URL: https://github.com/apache/doris/pull/69305#issuecomment-6285879660
<details>
<summary>TPC-DS: <b>Total hot run time: 150924 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 18a368d0494c850fb01eba1ff0269eaf09f70f05, data reload: false
query5 4309 590 465 465
query6 508 220 203 203
query7 4814 455 237 237
query8 334 171 155 155
query9 8822 3121 3113 3113
query10 518 320 270 270
query11 6031 4105 3606 3606
query12 162 94 93 93
query13 1258 439 319 319
query14 6127 3940 3676 3676
query14_1 3426 3435 3432 3432
query15 211 195 181 181
query16 952 423 375 375
query17 849 656 545 545
query18 2423 425 311 311
query19 206 168 135 135
query20 88 85 86 85
query21 219 127 109 109
query22 12987 12999 12747 12747
query23 13446 12886 12455 12455
query23_1 12619 12577 12523 12523
query24 7493 806 476 476
query24_1 473 467 468 467
query25 545 435 364 364
query26 1264 260 138 138
query27 2769 439 273 273
query28 4622 1968 1949 1949
query29 1576 606 462 462
query30 322 227 184 184
query31 1016 943 822 822
query32 179 99 95 95
query33 608 315 248 248
query34 961 843 506 506
query35 744 758 647 647
query36 859 814 746 746
query37 124 101 87 87
query38 1783 1752 1669 1669
query39 802 821 758 758
query39_1 748 742 743 742
query40 222 126 96 96
query41 71 69 67 67
query42 90 85 87 85
query43 341 364 316 316
query44 1303 683 690 683
query45 224 177 163 163
query46 813 955 566 566
query47 3011 2894 2879 2879
query48 287 298 206 206
query49 573 396 293 293
query50 675 269 206 206
query51 10339 10578 10458 10458
query52 88 78 76 76
query53 184 203 152 152
query54 266 197 178 178
query55 117 66 64 64
query56 240 206 226 206
query57 1679 1638 1579 1579
query58 309 257 253 253
query59 2075 2099 1920 1920
query60 292 241 226 226
query61 148 146 145 145
query62 398 333 292 292
query63 190 158 158 158
query64 2778 946 763 763
query65 3294 3277 3267 3267
query66 1782 413 296 296
query67 20112 20162 19895 19895
query68 2972 981 600 600
query69 396 295 252 252
query70 837 743 765 743
query71 297 225 214 214
query72 2705 2540 2328 2328
query73 523 515 288 288
query74 4679 4510 4368 4368
query75 2374 2279 1927 1927
query76 2306 1011 583 583
query77 344 383 264 264
query78 9809 9777 9203 9203
query79 1026 849 516 516
query80 773 416 326 326
query81 576 320 271 271
query82 957 133 104 104
query83 300 193 180 180
query84 319 123 100 100
query85 874 439 363 363
query86 439 241 266 241
query87 2034 1970 1802 1802
query88 3711 2639 2621 2621
query89 340 278 237 237
query90 1780 165 164 164
query91 160 152 128 128
query92 98 91 89 89
query93 992 983 561 561
query94 522 323 266 266
query95 564 381 301 301
query96 649 480 249 249
query97 2424 2418 2291 2291
query98 169 155 169 155
query99 780 766 649 649
Total cold run time: 235597 ms
Total hot run time: 150924 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