Re: SQL/JSON features for v15

On Mon, Aug 22, 2022 at 9:52 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> > Andres, Robert: Do these changes address your concerns about the use of
> > substransactions and reduce the risk of xid wraparound?
>
> Andres, Andrew, Amit, Robert -- as you have either worked on this or
> expressed opinions -- any thoughts on this current patch set?

I do not think that using subtransactions as part of the expression
evaluation process is a sound idea pretty much under any
circumstances. Maybe if the subtransations aren't commonly created and
don't usually get XIDs there wouldn't be a big problem in practice,
but it's an awfully heavyweight operation to be done inside expression
evaluation even in corner cases. I think that if we need to make
certain operations that would throw errors not throw errors, we need
to refactor interfaces until it's possible to return an error
indicator up to the appropriate level, not just let the error be
thrown and catch it.

The patches in question are thousands of lines of new code that I
simply do not have time or interest to review in detail. I didn't
commit this feature, or write this feature, or review this feature.
I'm not familiar with any of the code. To really know what's going on
here, I would need to review not only the new patches but also all the
code in the original commits, and probably some of the preexisting
code from before those commits that I have never examined in the past.
That would take me quite a few months even if I had absolutely nothing
else to do. And because I haven't been involved in this patch set in
any way, I don't think it's really my responsibility.

At the end of the day, the RMT is going to have to take a call here.
It seems to me that Andres's concerns about code quality and lack of
comments are probably somewhat legitimate, and in particular I do not
think the use of subtransactions is a good idea. I also don't think
that trying to fix those problems or generally improve the code by
committing thousands of lines of new code in August when we're
targeting a release in September or October is necessarily a good
idea. But I'm also not in a position to say that the project is going
to be irreparably damaged if we just ship what we've got, perhaps
after fixing the most acute problems that we currently know about.
This is after all relatively isolated from the rest of the system.
Fixing the stuff that touches the core executor is probably pretty
important, but beyond that, the worst thing that happens is the
feature sucks and people who try to use it have bad experiences. That
would be bad, and might be a sufficient reason to revert, but it's not
nearly as bad as, say, the whole system being slow, or data loss for
every user, or something like that. And we do have other bad code in
the system. Is this a lot worse? I'm not in a position to say one way
or the other.

-- 
Robert Haas
EDB: http://www.enterprisedb.com