Open morgo opened 2 years ago
@morgo Audit log are using StmtCtx
to read the SQL, StmtType, etc.. However the ResetContextOfStmt
is called in ExecuteStmt
so when we call OnGeneralEvent
with event Starting
, the StmtCtx
is not properly set yet.
It is not easy to move ResetContextOfStmt
from ExecuteStmt
to handleStmt
because ExecuteStmt
is also called by other methods like ExecRestrictedStmt
or ExecuteInternal
. If we move it, the modification will be huge.
Maybe it is not a good idea to force the audit log plugin to read the context from StmtCtx
. I think we can decouple audit log with StmtCtx
and pass the context to audit log according to the environment.
The current plugin API of tidb is also not well designed and does not provide a clear semantic for statement's lifecycle. So we are considering to redesign the audit log in the future.
Bug Report
Please answer these questions before submitting your issue. Thanks!
1. Minimal reproduce step (Required)
Write a simple plugin based on https://github.com/pingcap/tidb/tree/master/plugin/conn_ip_example
For the
OnGeneralEvent
use the following code:Compile the server, and run two statements:
2. What did you expect to see? (Required)
For the second statement, there should be a
Starting
andCompleted
event. The starting event should have the SQL ofselect 1
.3. What did you see instead (Required)
The
Starting
event incorrectly has all the context of the previously executed statement (show tables). There is no context about the statement which isselect 1
. This is the same for the eventError
, where if I were to create an invalid statement, all the context refers to the previously executed statement.Here is a printf:
4. What is your TiDB version? (Required)
master