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