Repository: ClickHouse/ClickHouse
File: src/Access/AccessRights.cpp (C++)

Code before:
            for (const auto & this_child : *children)
            {
                Node other_child = other.tryGetLeaf(this_child.node_name, this_child.level, !this_child.isLeaf());
                if (!this_child.contains(other_child))
                    return false;
            }
        }

        return true;
    }

    void makeUnion(const Node & other)
    {
        /// We need these tmp nodes because union/intersect operations use the `getLeaf` function, which can't be made const.
        /// Potentially, we can use tryGetLeaf, but it's very complicated to traverse both trees at the same time:
        ///
        /// Tree1:
        ///              f (SELECT)
        ///           /            \
        ///        ancy (SELECT)   oo (SELECT)
        ///          |                 |
        ///    "" (SELECT, INSERT)    "" (SELECT, INSERT)
        ///
        /// Tree2:
        ///          foo (SELECT)
        ///              |
        ///         "" (SELECT, INSERT)
        ///
        /// Here Tree2.getLeaf("f") will return a valid node, but Tree2.tryGetLeaf("f") will return nullptr

        Node result;
        Node rhs = other;
        makeUnionRec(result, rhs);
        children = std::move(result.children);
        flags |= other.flags;
        optimizeTree();
    }

    void makeIntersection(const Node & other)
    {
        Node result;
        Node rhs = other;
        makeIntersectionRec(result, rhs);
        children = std::move(result.children);
        flags &= other.flags;
        optimizeTree();
    }

    void makeDifference(const Node & other)
    {
        Node rhs = other;

Review comment (suggestion):
This implementation performs `tryGetLeaf()` lookups inside loops over children on both sides, and then recurses, which can lead to repeated linear scans (notably since `tryGetLeaf()` searches children via `find_if`). For large grants trees, this can become quadratic (or worse) in the number of nodes. If `contains()` is used in hot/admin paths (e.g., backup restore validation across many objects), consider reducing repeated lookups by iterating children in a way that avoids per-child linear search (e.g., caching an index for the current node’s children for the duration of `contains()`, or using a lookup-friendly structure where appropriate).
```suggestion

```

Code after:
        /// Step 1 alone would pass because `other` has no children to check against, but `this` does not contain `other` because
        /// `this` has revoked SELECT on "foo".

        if (!flags.contains(other.flags))
            return false;

        if (other.children)
        {
            for (const auto & other_child : *other.children)
            {
                Node this_child = tryGetLeaf(other_child.node_name, other_child.level, !other_child.isLeaf());
                if (!this_child.contains(other_child))
                    return false;
            }
        }

        if (children)
        {
            for (const auto & this_child : *children)
            {
                Node other_child = other.tryGetLeaf(this_child.node_name, this_child.level, !this_child.isLeaf());
                if (!this_child.contains(other_child))
                    return false;
            }
        }

        return true;
    }

    void makeUnion(const Node & other)
    {
        /// We need these tmp nodes because union/intersect operations use the `getLeaf` function, which can't be made const.
        /// Potentially, we can use tryGetLeaf, but it's very complicated to traverse both trees at the same time:
        ///
        /// Tree1:
        ///              f (SELECT)
        ///           /            \
        ///        ancy (SELECT)   oo (SELECT)
        ///          |                 |
        ///    "" (SELECT, INSERT)    "" (SELECT, INSERT)
        ///
        /// Tree2:
        ///          foo (SELECT)
        ///              |
        ///         "" (SELECT, INSERT)
        ///
        /// Here Tree2.getLeaf("f") will return a valid node, but Tree2.tryGetLeaf("f") will return nullptr

        Node result;
        Node rhs = other;
        makeUnionRec(result, rhs);