Add a new system table user_query_log#104462
Conversation
alexey-milovidov
added
the
can be tested
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
niyue
force-pushed
the
feat/user-query-log
branch
from
ea9e2d4 to
bca687e
Compare
|
This PR is ready for review, could you please help? Thanks. |
|
The |
… query logs safely.
Introduce a security barrier mode that prevents view inlining and outer filter pushdown, and exposes barrier view columns without wrappers. Use it so user-supplied predicates cannot evaluate against filtered-out values.
Expose `attachSystemUserQueryLog` so `clickhouse-local` can create `system.user_query_log` after `initializeSystemLogs` creates `system.query_log`.
Add the required `current_database = currentDatabase()` predicate to `system.query_log` and `system.user_query_log` queries in the stateless test, so the query log style check accepts the test and concurrent test logs are ignored. Issue: ClickHouse#82539
…g is configured in a non system database.
niyue
force-pushed
the
feat/user-query-log
branch
from
01c8e4d to
a6045a2
Compare
|
@alexey-milovidov Thanks for the guidance. I've updated the branch with the latest master. |
niyue
force-pushed
the
feat/user-query-log
branch
from
460b718 to
ffd3010
Compare
# Conflicts: # src/Storages/System/attachSystemTables.cpp # src/Storages/System/attachSystemTables.h
|
Updated the branch with the latest For the record, the two red checks on the previous CI run were not caused by this PR:
The |
Replace debug-only `assert` checks in the system view attachment path with `chassert` so the code follows the ClickHouse assertion style.
Mark generated `StorageView` instances attached as system views as system storage, and rebuild `CREATE VIEW` metadata from in-memory view metadata for system view introspection. Add coverage for `SHOW CREATE TABLE system.user_query_log` and `system.tables.as_select`.
Reject user-created or renamed objects at the reserved system table name even when the configured view is disabled or not attached. Add coverage for `CREATE TABLE` and `RENAME TABLE` attempts that target the reserved name.
Reject non-system storage in the common `attachTable` path when it targets the reserved filtered user query log view name, so startup metadata loading also fails closed. Add coverage for a legacy `metadata/system/user_query_log.sql` file loaded by `clickhouse-local`. Related: ClickHouse#104462
Run the filtered system view without a `default` definer so `StorageView` does not depend on a mutable user identity. Validate the view metadata as `SQL SECURITY NONE` and extend introspection coverage to check the missing definer.
Update the system table documentation to describe `SQL SECURITY NONE` instead of the removed `default` definer dependency.
Use `initial_user` when it is present so distributed query-log rows remain visible to the original caller and are not exposed to the remote execution user. Keep `user` as the fallback for rows without an initiating user.
Avoid emitting `PREWHERE` in the view definition because the configured `query_log.engine` can use storages such as `Null` that do not support `PREWHERE`. A normal `WHERE` predicate keeps those engines usable while still allowing `MergeTree` to optimize the filter. Add a stateless regression test for `ENGINE = Null` query log storage.
Build the view comment from the configured query log table so `system.tables.comment` matches the actual `SELECT` source when `query_log.table` is customized. Extend the `ENGINE = Null` regression test to use a custom query log table and check the generated comment.
Drop the documentation sentence that said the view filter uses `PREWHERE`. The view now uses `WHERE`, and the exact clause is an implementation detail already covered by `SHOW CREATE TABLE`.
Resolve the `DatabaseOnDisk` merge conflict by keeping the view-specific metadata handling while preserving the upstream `metadata_ptr` comment path.
Prevent `additional_table_filters` from being applied through `system.user_query_log`, where filters targeting the underlying `system.query_log` table could observe rows hidden by the view predicate. Add a regression test that verifies such filters are rejected explicitly instead of being evaluated inside the security barrier.
Block `additional_result_filter` before planner filter collection can apply it through security barrier views, and reject it again when reading the view body. Extend `system.user_query_log` coverage for both analyzer paths.
The Style check `various_checks.sh` requires every test that mentions `system.query_log`/`system.query_thread_log` to include a `current_database = currentDatabase()` condition. Test `04402_user_query_log_additional_filters.sh` referenced `system.query_log` only as the `additional_table_filters` map key (the source table of the `system.user_query_log` view), which tripped the heuristic. Add the predicate to the three `system.user_query_log` queries. The queries are still rejected by the security-barrier view before reading any data, so the test outcome is unchanged, and the predicate now also isolates the view read by the test database. CI report: ClickHouse#104462
| if (security_barrier) | ||
| { | ||
| const auto & settings = context->getSettingsRef(); | ||
| if (!settings[Setting::additional_table_filters].value.empty()) |
There was a problem hiding this comment.
This blocks every non-empty additional_table_filters map, but the side channel only exists for entries that can resolve inside the barrier view (for system.user_query_log, keys like system.query_log or the configured source table). additional_table_filters is table-scoped in parseAdditionalFilterConditionForTable, so safe cases now regress too: for example, SELECT ... FROM system.user_query_log u JOIN system.one o ... SETTINGS additional_table_filters = {'system.one': 'dummy = 0'} or ... SETTINGS additional_table_filters = {'system.user_query_log': 'event_date = today()'} will fail here even though those predicates only apply outside the barrier. Please reject or strip only keys that target the inner source table instead of banning the whole query.
|
|
||
| <SystemLogParameters/> | ||
|
|
||
| The `enable_user_query_log` parameter creates [`system.user_query_log`](/operations/system-tables/user_query_log), a view over the query log table filtered by `currentUser`. It is enabled by default. |
There was a problem hiding this comment.
This setting description is stale again. The final implementation does not always read from system.query_log, and it no longer filters with just user = currentUser(): it reads from the configured query_log.database / query_log.table and uses the initiating-user predicate if(initial_user != '', initial_user, user) = currentUser(). Please keep this page aligned with docs/en/operations/system-tables/user_query_log.md so operators do not get contradictory behavior docs.
LLVM Coverage ReportChanged lines: Changed C/C++ lines covered: 208/251 (82.87%) · Uncovered code |
3 participants
Changelog category (leave one):
Changelog entry:
system.user_query_logsystem table that allows users to read only their own query log records without direct access tosystem.query_log.Documentation entry for user-facing changes
This PR will address #82539