| From: | Nikita Malakhov <hukutoc(at)gmail(dot)com> |
|---|---|
| To: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
| Cc: | Amit Langote <amitlangote09(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: SQL/JSON json_table plan clause |
| Date: | 2026-05-11 20:12:53 |
| Message-ID: | CAN-LCVMvrkaNM+pUN=KvoJ4_c6yCVaW5gNucQWWNtsAmMxzOCA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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 <aekorotkov(at)gmail(dot)com>
wrote:
> Hi, Nikita!
>
> On Mon, Mar 16, 2026 at 6:30 PM Nikita Malakhov <hukutoc(at)gmail(dot)com> 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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-05-11 20:40:18 | Re: COPY ON_CONFLICT TABLE; save duplicated record to another table. |
| Previous Message | Baji Shaik | 2026-05-11 19:45:34 | [PATCH] Fix psql tab completion for REPACK boolean options |