Implement confidential statement #4910
Conversation
tools/shell/embedded_shell.cpp
Outdated
| auto isSuccess = queryResult->isSuccess(); | ||
| queryResults.push_back(std::move(queryResult)); | ||
| if (isSuccess && !database->getConfig().readOnly) { | ||
| auto clientContext = conn->getClientContext(); |
There was a problem hiding this comment.
This looks fine to me. @ray6080 can u take a second look?
There was a problem hiding this comment.
hmm I'm concerned about this approach because it introduces an additional binding step to every write query, which I don't think the overhead is insignificant. Need to think a bit on what's an alternative way.
There was a problem hiding this comment.
The additional binding only occurs in the shell and it should only take a few milliseconds. I don't think CLI users care about this little time.
There was a problem hiding this comment.
But I think we can limit this to a smaller set? At least we can limit this to call functions only, which you can check QuerySummary::preparedSummary.
Open for discussion here, if it is possible to introduce a new statement type for credential related queries. so this can be avoided at all (we just need to check the statement type), and easily extended to other kinds of queries in the future too.
acquamarin
force-pushed
the
confidential-statement
branch
from
2947521 to
fec6e29
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4910 +/- ##
==========================================
+ Coverage 86.56% 86.58% +0.01%
==========================================
Files 1406 1409 +3
Lines 60831 60859 +28
Branches 7483 7483
==========================================
+ Hits 52661 52696 +35
+ Misses 8001 7994 -7
Partials 169 169 ☔ View full report in Codecov by Sentry. |
Benchmark ResultMaster commit hash:
|
This PR introduces the confidential statement.
A statement is considered as confidential if it contains user's secrets (password, key).
Shell is not going to record those statements;
Fixes #4891