On Wed, Apr 3, 2024 at 7:34 PM Michael Paquier <michael@paquier.xyz> wrote:
> I've been re-reading the patch again to remember what this is about,
> and I'm OK with having this "path" column in the catalog. However,
> I'm somewhat confused by the choice of having a temporary number that
> shows up in the catalog representation, because this may not be
> constant across multiple calls so this still requires a follow-up
> temporary ID <-> name mapping in any SQL querying this catalog. A
> second thing is that array does not show the hierarchy of the path;
> the patch relies on the order of the elements in the output array
> instead.
This complaint doesn't seem reasonable to me. The point of the path,
as I understand it, is to allow the caller to make sense of the
results of a single call, which is otherwise impossible. Stability
across multiple calls would be much more difficult, particularly
because we have no unique, long-lived identifier for memory contexts,
except perhaps the address of the context. Exposing the pointer
address of the memory contexts to clients would be an extremely bad
idea from a security point of view -- and it also seems unnecessary,
because the point of this function is to get a clear snapshot of
memory usage at a particular moment, not to track changes in usage by
the same contexts over time. You could still build the latter on top
of this if you wanted to do that, but I don't think most people would,
and I don't think the transient path IDs make it any more difficult.
I feel like Melih has chosen a simple and natural representation and I
would have done pretty much the same thing. And AFAICS there's no
reasonable alternative design.
--
Robert Haas
EDB: http://www.enterprisedb.com