[Feature] (sql digest) support sql digest #8919
Conversation
| protected abstract String toSqlImpl(); | ||
|
|
||
| protected String toDigestImpl() { | ||
| return toSqlImpl(); |
There was a problem hiding this comment.
Maybe we should throw NotImplementedException here?
So that we can ensure that every newly added Expr subclass will override this method?
There was a problem hiding this comment.
Thx for your reply.
The abstract method toSqlImpl() will be overrided in each subclass. So there is no need to throw out exceptions.
The only situation is that a newly added subclass, which its sql digest needs to be different from sql impl, should override this method.
Added comments over this method to remind developers.
github-actions
bot
added
the
area/planner
morningman
added
the
kind/feature
| @Override | ||
| public String toDigestImpl() { | ||
| StringBuilder sb = new StringBuilder(); | ||
| sb.append(fnCall.toDigest()).append(" OVER ("); |
There was a problem hiding this comment.
I think we don't need those OVER or PARTITION BY keywords in digest?
maybe just a prefix like AnalyticExpr is enough.
There was a problem hiding this comment.
And I'm not quite sure about the performance impact of this implementation.
Looks like these are too much string operations.
Have you test its performance? Especially in high concurrency senario.
And I found some reference about how to implement digest or fingerprint of a SQL:
https://segmentfault.com/a/1190000040132966
There was a problem hiding this comment.
Thx.
I've tested locally, there is a 10% query performance loss. It looks bad.
So decide to put it in auditAfterExec() and only for slow queries.
morningman
left a comment
morningman
left a comment
There was a problem hiding this comment.
Also need to update audit loader plugin:
fe_plugins/auditloader/src/main/java/org/apache/doris/plugin/audit/AuditLoaderPlugin.java
fe_plugins/auditloader/src/main/java/org/apache/doris/plugin/audit/DorisStreamLoader.java
Henry2SS
force-pushed
the
whz-sql-digest
branch
from
8099363 to
8ea1c39
Compare
Done. |
github-actions
bot
added
the
kind/docs
|
please fix conflicts ❤️ |
Done~ |
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
please fix fe code style error, thanks |
|
@Henry2SS @morrySnow @morningman Does Doris 2.1.5 have a configuration parameter to enable the recording of all SQL sql-digests in the audit log? |
4 participants
Proposed changes
Issue Number: close #8918
Problem Summary:
Describe the overview of changes.
Checklist(Required)
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...