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 v16Based 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
OpenPGP_signature
Description: OpenPGP digital signature
