Hi Alexander, Thank you for the review! I'll try to correct noted points asap.
On Mon, May 11, 2026 at 3:42 PM Alexander Korotkov <[email protected]> wrote: > Hi, Nikita! > > On Mon, Mar 16, 2026 at 6:30 PM Nikita Malakhov <[email protected]> wrote: > > I've rebased patch set onto current master and have corrected > > output according to recent changes in JSON array processing. > > Awaiting review. > > Thank you for your efforts in this direction. I've reviewed the v22 of > the patch, and I have the following notes. > 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. > 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. > 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). > 4. Unused typedefs in src/tools/pgindent/typedefs.list: > JsonTableScanState, JsonPathSpec, JsonTablePlanStateType, > JsonTableJoinState. > 5. Empty comment in JsonTablePlanState definition. Pointed by Amit, > but not fixed. > 6. Rename passingArgs to passing_Args for no reason in parse_jsontable.c. > 7. Patch lacks documentation (also pointed by Amit) > 8. Patch could use pgindent run. > > ------ > Regards, > Alexander Korotkov > Supabase > -- Regards, Nikita Malakhov Postgres Professional The Russian Postgres Company https://postgrespro.ru/
