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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
