For new files introduced in the patches:
+ * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
2021 is a few days ahead. It would be good to update the year range.
For transformJsonTableColumn:
+ jfexpr->op =
+ jtc->coltype == JTC_REGULAR ? IS_JSON_VALUE :
+ jtc->coltype == JTC_EXISTS ? IS_JSON_EXISTS : IS_JSON_QUERY;
Should IS_JSON_EXISTS be mentioned in the comment preceding the method ?
For JsonTableDestroyOpaque():
+ state->opaque = NULL;
Should the memory pointed to by opaque be freed ?
+ l2 = list_head(tf->coltypes);
+ l3 = list_head(tf->coltypmods);
+ l4 = list_head(tf->colvalexprs);
Maybe the ListCell pointer variables can be named corresponding to the
lists they iterate so that the code is easier to understand.
+typedef enum JsonTablePlanJoinType
+{
+ JSTP_INNER = 0x01,
+ JSTP_OUTER = 0x02,
+ JSTP_CROSS = 0x04,
Since plan type enum starts with JSTP_, I think the plan join type should
start with JSTPJ_ or JSTJ_. Otherwise the following would be a bit hard to
read:
+ else if (plan->plan_type == JSTP_JOINED)
+ {
+ if (plan->join_type == JSTP_INNER ||
+ plan->join_type == JSTP_OUTER)
since different fields are checked in the two if statements but the
prefixes don't convey that.
+ Even though the path names are not incuded into the <literal>PLAN
DEFAULT</literal>
Typo: incuded -> included
+ int nchilds = 0;
nchilds -> nchildren
+#if 0 /* XXX it' unclear from the standard whether root path name is
mandatory or not */
+ if (plan && plan->plan_type != JSTP_DEFAULT && !rootPathName)
Do you plan to drop the if block ?
Cheers
On Fri, Dec 25, 2020 at 12:32 PM Nikita Glukhov <n.glu...@postgrespro.ru>
wrote:
>
> On 03.08.2020 10:55, Michael Paquier wrote:
>
> On Sun, Jul 05, 2020 at 12:15:58PM -0500, Justin Pryzby wrote:
>
> It looks like this needs to be additionally rebased - I will set cfbot to
> "Waiting".
>
> ... Something that has not happened in four weeks, so this is marked
> as returned with feedback. Please feel free to resubmit once a rebase
> is done.
> --
> Michael
>
>
> Atatched 44th version of the pacthes rebased onto current master
> (#0001 corresponds to v51 of SQL/JSON patches).
>
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
