Avoid deep copy of AccessRights in getGrantQueriesImpl when it is only read

Project: ClickHouse/ClickHouse | Language: C++ | Concepts: algorithmic-optimization, data-structures, memory-management, testing-strategy | Difficulty: mid
Link: https://github.com/ClickHouse/ClickHouse/pull/115696

The problem

The function getGrantQueriesImpl always made a deep copy of a user's AccessRights tree, even when it only needed to read from it. Since AccessRights is a radix tree, copying it is O(number of nodes). For users with thousands of grants, this caused severe slowdowns (seconds per GRANT/SHOW GRANTS), excessive CPU usage, and high memory allocations.

Why it's non-trivial

A naive fix might simply avoid copying altogether, but the function sometimes needs to mutate the tree (when final == true). Avoiding the copy in all cases would break correctness. The code must distinguish between read-only and mutating cases and only avoid the copy when safe.

The approach

The PR changes the logic in getGrantQueriesImpl to only copy the AccessRights tree when mutation is required. Otherwise, it passes a reference, avoiding the expensive deep copy.

Old code:

-        AccessRights access = grantee.access;
-        if (final)
-            unionAccessFromRoles(access, grantee, access_control);
-
-        getGrantsFromAccess(res, access, grantees, access_control, attach_mode, with_implicit);

This always copies the tree (AccessRights access = grantee.access;), then possibly mutates it.

New code:

+        AccessRights access_copy;
+
+        if (final)
+        {
+            access_copy = grantee.access;
+            unionAccessFromRoles(access_copy, grantee, access_control);
+        }
+
+        const AccessRights & access = final ? access_copy : grantee.access;
+
+        getGrantsFromAccess(
+            res,
+            access,
+            grantees,
+            access_control,
+            attach_mode,
+            with_implicit);

• If final is true, it copies and mutates as before.
• If final is false, it passes the original tree by reference, avoiding the deep copy.

Test coverage:
A new shell test ensures grants/revokes work with many per-table grants, and that the number of grants is as expected:

for i in $(seq 1 30); do
    # ... create tables and grant privileges ...
done
$CLICKHOUSE_CLIENT -q "SHOW GRANTS FOR $TEST_USER" | awk 'END {print NR}'

This confirms the optimization does not break correctness.

From the review thread

No notable review discussion on this PR.

Takeaways

• Avoid unnecessary deep copies of large data structures when only read access is needed; use references where safe.
• Always distinguish between cases where mutation is required and where it is not, to optimize for the common case.
• When optimizing internals, add regression tests to confirm behavior is unchanged for both normal and edge cases.