From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Amit Langote <amitlangote09(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> |
Subject: | Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size |
Date: | 2022-08-09 18:57:25 |
Message-ID: | 20220809185725.t477tp54v3qv4uyi@awork3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2022-08-09 14:04:48 -0400, Jonathan S. Katz wrote:
> On 8/9/22 11:03 AM, Andrew Dunstan wrote:
> >
> > On 2022-08-09 Tu 09:59, Jonathan S. Katz wrote:
>
> > > The RMT met today to discuss the state of this open item surrounding
> > > the SQL/JSON feature set. We discussed the specific concerns raised
> > > about the code and debated four different options:
> > >
> > > 1. Do nothing and continue with the community process of stabilizing
> > > the code without significant redesign
> > >
> > > 2. Recommend holding up the v15 release to allow for the code to be
> > > redesigned and fixed (as based on Andres' estimates, this would push
> > > the release out several months).
Obviously that's a question of the resources brought to bear.
From my angle: I've obviously some of my own work to take care of as well, but
it's also just hard to improve something that includes a lot of undocumented
design decisions.
> > > 3. Revert the problematic parts of the code but try to include some
> > > of the features in the v15 release (e.g. JSON_TABLE)
> > I very much doubt option 3 is feasible. The parts that are controversial
> > go back at least in part to the first patches of the series. Trying to
> > salvage anything would almost certainly be more disruptive than trying
> > to fix it.
Agreed.
> > > 4. Revert the feature set and redesign and try to include for v16
> > >
> > > Based on the concerns raised, the RMT is recommending option #4, to
> > > revert the SQL/JSON changes for v15, and come back with a redesign for
> > > v16.
> > >
> > > If folks think there are some bits we can include in v15, we can
> > > consider option #3. (Personally, I would like to see if we can keep
> > > JSON_TABLE, but if there is too much overlap with the problematic
> > > portions of the code I am fine with waiting for v16).
> > >
> > > At this stage in the release process coupled with the concerns, we're
> > > a bit worried about introducing changes that are unpredictable in
> > > terms of stability and maintenance. We also do not want to hold up the
> > > release while this feature set is goes through a redesign without
> > > agreement on what such a design would look like as well as a timeline.
> > >
> > > Note that the above are the RMT's recommendations; while the RMT can
> > > explicitly call for a revert, we want to first guide the discussion on
> > > the best path forward knowing the challenges for including these
> > > features in v15.
Unless we decide on 4 immediately, I think it might be worth starting a
separate thread to get more attention. The subject doesn't necessarily have
everyone follow along.
> > I'm not sure what the danger is to stability, performance or correctness
> > in applying the changes Amit has proposed for release 15. But if that
> > danger is judged to be too great then I agree we should revert.
My primary problem is that as-is the code is nearly unmaintainable. It's hard
for Amit to fix that, given that he's not one of the original authors.
> Rereading the thread for the umpteenth time, I have seen Amit working
> through Andres' concerns. From my read, the ones that seem pressing are:
>
> * Lack of design documentation, which may be leading to some of the general
> design concerns
> * The use of substransactions within the executor, though docs explaining
> the decisions on that could alleviate it (I realize this is a big topic and
> any summary I give won't capture the full nuance)
I don't think subtransactions per-se are a fundamental problem. I think the
error handling implementation is ridiculously complicated, and while I started
to hack on improving it, I stopped when I really couldn't understand what
errors it actually needs to handle when and why.
> * Debate on how to handle the type coercions
That's partially related to the error handling issue above.
One way this code could be drastically simplified is to force all
type-coercions to go through the "io coercion" path, which could be
implemented as a single execution step (which thus could trivially
start/finish a subtransaction) and would remove a lot of the complicated code
around coercions.
> > I should add that I'm very grateful to Amit for his work, and I'm sure
> > it isn't wasted effort, whatever the decision.
>
> +1. While I've been quiet on this thread to date, I have definitely seen
> Amit working hard on addressing the concerns.
+1
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-08-09 19:17:44 | Re: PG 15 (and to a smaller degree 14) regression due to ExprEvalStep size |
Previous Message | David Steele | 2022-08-09 18:49:39 | Re: moving basebackup code to its own directory |