On Tue, 16 Jul 2024 at 06:19, Robert Haas <robertmh...@gmail.com> wrote: > I'm not against what you're trying to do here, but I feel like you > might be over-engineering it. I don't think there was anything really > wrong with what Melih was doing, and I don't think there's anything > really wrong with converting the path to an array of strings, either. > Sure, it might not be perfect, but future patches could always remove > the name duplication. This is a debugging facility that will be used > by a tiny minority of users, and if some non-uniqueness gets > reintroduced in the future, it's not a critical defect and can just be > fixed when it's noticed.
I'm just not on board with the query-returns-correct-results-most-of-the-time attitude and I'm surprised you are. You can get that today if you like, just write a WITH RECURSIVE query joining "name" to "parent". If the consensus is that's fine because it works most of the time, then I don't see any reason to invent a new way to get equally correct-most-of-the-time results. > That said, if you want to go with the integer > IDs and want to spend more time massaging it, I also think that's > fine. I simply don't believe it's the only way forward here. YMMV, but > my opinion is that none of these approaches have such critical flaws > that we need to get stressed about it. If there are other ways forward that match the goal of having a reliable way to determine the parent of a MemoryContext, then I'm interested in hearing more. I know you've mentioned about having unique names, but I don't know how to do that. Do you have any ideas on how we could enforce the uniqueness? I don't really like your idea of renaming contexts when we find duplicate names as bug fixes. The nature of our code wouldn't make it easy to determine as some reusable code might create a context as a child of CurrentMemoryContext and multiple callers might call that code within a different CurrentMemoryContext. One problem is that, if you look at MemoryContextCreate(), we require that the name is statically allocated. We don't have the flexibility to assign unique names when we find a conflict. If we were to come up with a solution that assigned a unique name, then I'd call that "over-engineered" for the use case we need it for. I think if we did something like that, it would undo some of the work Tom did in 442accc3f. Also, I think it was you that came up with the idea of MemoryContext reuse (9fa6f00b1)? Going by that commit message, it seems to be done for performance reasons. If MemoryContext.name was dynamic, there'd be more allocation work to do when reusing a context. That might undo some of the performance gains seen in 9fa6f00b1. I don't really want to go through the process of verifying there's no performance regress for a patch that aims to make pg_backend_memory_contexts more useful. David
