On 8/5/22 4:36 PM, Andres Freund wrote:
> Hi,
>
> I tried to look into some of the questions from Amit, but I have e.g. no idea
> what exactly the use of subtransactions tries to achieve - afaics 1a36bc9dba8
> is the first patch to introduce needing to evaluate parts expressions in a
> subtransaction - but there's not a single comment explaining why.
>
> It's really hard to understand the new json code. It's a substantial amount of
> new infrastructure, without any design documentation that I can find. And it's
> not like it's just code that's equivalent to nearby stuff. To me this falls
> way below our standards and I think it's *months* of work to fix.
>
> Even just the expresion evaluation code: EvalJsonPathVar(),
> ExecEvalJsonConstructor(), ExecEvalJsonExpr(), ExecEvalJson(). There's one
> layer of subtransactions in one of the paths in ExecEvalJsonExpr(), another in
> ExecEvalJson(). Some paths of ExecEvalJsonExpr() go through subtransactions,
> others don't.
>
> It's one thing for a small set of changes to be of this quality. But this is
> pretty large - a quick summing of diffstat ends up with about 17k lines added,
> of which ~2.5k are docs, ~4.8k are tests.
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).
3. Revert the problematic parts of the code but try to include some
of the features in the v15 release (e.g. JSON_TABLE)
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.
Thanks,
Jonathan
