github-actions[bot] commented on code in PR #64600: URL: https://github.com/apache/doris/pull/64600#discussion_r3425736657
########## regression-test/suites/audit/test_audit_log_hint_session_context.groovy: ########## @@ -0,0 +1,71 @@ +// 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. + +// A session variable changed by a per-query SET_VAR hint is temporary: it is reverted in +// StmtExecutor.execute()'s finally block. The audit log is built afterwards (in auditAfterExec), +// so without a pre-revert snapshot the hint value never reaches changed_variables. This case +// verifies that a per-query SET_VAR(session_context=...) is recorded in the audit log's +// changed_variables. The value is quoted so the hint parser keeps the ':' in the value. +suite("test_audit_log_hint_session_context", "nonConcurrent") { + try { + sql "set global enable_audit_plugin = true" + } catch (Exception e) { + log.warn("skip this case, because " + e.getMessage()) + assertTrue(e.getMessage().toUpperCase().contains("ADMIN")) + return + } + + def tbl = "audit_hint_session_context" + sql "drop table if exists ${tbl}" + sql """ + CREATE TABLE `${tbl}` (`id` bigint) ENGINE=OLAP + DUPLICATE KEY(`id`) + DISTRIBUTED BY HASH(`id`) BUCKETS 1 + PROPERTIES ("replication_allocation" = "tag.location.default: 1") + """ + sql "insert into ${tbl} values (1)" + + // unique markers so the exact statement can be located in the audit log + def hintTrace = "trace_id:hint_sc_7F3A2B" + def stmtMarker = "audit_hint_sc_marker_7F3A2B" + + sql "truncate table __internal_schema.audit_log" + + // per-query SET_VAR hint sets session_context for this statement only + sql """select /*+ SET_VAR(session_context="${hintTrace}") */ id, '${stmtMarker}' as marker from ${tbl}""" + + Thread.sleep(6000) + sql """call flush_audit_log()""" + + def retry = 180 + def query = """select ELEMENT_AT(changed_variables, 'session_context') + from __internal_schema.audit_log + where stmt like '%${stmtMarker}%' order by time desc limit 1""" Review Comment: This predicate can match the audit rows produced by the polling query itself, because the lookup SQL also contains `${stmtMarker}`. `flush_audit_log()` only flushes the loader's current buffer; if the target SELECT event is still queued and the retry loop waits for the background loader, later audit rows for this lookup can become the newest `%${stmtMarker}%` match and return a null `session_context`, making the test flaky. Please constrain the predicate to the target SET_VAR statement so the audit-table lookup cannot match itself. ```suggestion where stmt like 'select /*+ SET_VAR(session_context=%' and stmt like '%${stmtMarker}%' order by time desc limit 1""" ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [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]
