Hi hackers! Again, Alexander and Amit, thanks for the review. I've rebased the patch and made some changes according to Alexander's notes. I've slightly rearranged files between patches for it to be easier to review, so now there are 3 patches: v23-0001-json-table-plan-clause.patch - main code changes v23-0002-json-table-plan-tests.patch - test cases and out file changes v23-0003-json-table-plan-docs.patch - docs package
<...> >1. IsA(planstate, JsonTableSiblingJoin) is wrong. planstate is not a >node, thus IsA() can't be applied to it. You should instead do >IsA(planstate->plan, JsonTableSiblingJoin). It wasn't catched, >because regression tests don't exercise this branch. So, you also >need to improve the coverage. - done; >2. get_json_table() with patch uses JSON_BEHAVIOR_EMPTY as the default >value for deparsing, while parsing still uses >JSON_BEHAVIOR_EMPTY_ARRAY. Looks plain wrong. I'm not sure what is >intention here. - looks like leftover from older version, changed to default behavior, so unnecessary emission of ERROR is omitted; >3. PLAN clause is always emitted during deparsing even if user didn't >specify anything. I would prefer to skip PLAN clause in this case >unless there is strong reason to do the opposite (this reason must be >pointed if any). - done, this made test cases much more readable; >4. Unused typedefs in src/tools/pgindent/typedefs.list: >JsonTableScanState, JsonPathSpec, JsonTablePlanStateType, >JsonTableJoinState. - done; >5. Empty comment in JsonTablePlanState definition. Pointed by Amit, >but not fixed. - done; >6. Rename passingArgs to passing_Args for no reason in parse_jsontable.c. - done; >7. Patch lacks documentation (also pointed by Amit) - done, documentation is provided in separate patch; >8. Patch could use pgindent run. - not done yet but would provide a newer version with it. -- Regards, Nikita Malakhov Postgres Professional The Russian Postgres Company https://postgrespro.ru/
v23-0001-json-table-plan-clause.patch
Description: Binary data
v23-0002-json-table-plan-tests.patch
Description: Binary data
v23-0003-json-table-plan-docs.patch
Description: Binary data
