On Fri, Mar 22, 2024 at 12:08 AM Amit Langote <amitlangot...@gmail.com> wrote: > > On Wed, Mar 20, 2024 at 9:53 PM Amit Langote <amitlangot...@gmail.com> wrote: > > I'll push 0001 tomorrow. > > Pushed that one. Here's the remaining JSON_TABLE() patch. >
hi. minor issues i found json_table patch. + if (!IsA($5, A_Const) || + castNode(A_Const, $5)->val.node.type != T_String) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("only string constants are supported in JSON_TABLE" + " path specification"), + parser_errposition(@5)); as mentioned in upthread, this error message should be one line. +const TableFuncRoutine JsonbTableRoutine = +{ + JsonTableInitOpaque, + JsonTableSetDocument, + NULL, + NULL, + NULL, + JsonTableFetchRow, + JsonTableGetValue, + JsonTableDestroyOpaque +}; should be: const TableFuncRoutine JsonbTableRoutine = { .InitOpaque = JsonTableInitOpaque, .SetDocument = JsonTableSetDocument, .SetNamespace = NULL, .SetRowFilter = NULL, .SetColumnFilter = NULL, .FetchRow = JsonTableFetchRow, .GetValue = JsonTableGetValue, .DestroyOpaque = JsonTableDestroyOpaque }; +/* + * JsonTablePathSpec + * untransformed specification of JSON path expression with an optional + * name + */ +typedef struct JsonTablePathSpec +{ + NodeTag type; + + Node *string; + char *name; + int name_location; + int location; /* location of 'string' */ +} JsonTablePathSpec; the comment still does not explain the distinction between "location" and "name_location"? JsonTablePathSpec needs to be added to typedefs.list. JsonPathSpec should be removed from typedefs.list. +/* + * JsonTablePlanType - + * flags for JSON_TABLE plan node types representation + */ +typedef enum JsonTablePlanType +{ + JSTP_DEFAULT, + JSTP_SIMPLE, + JSTP_JOINED, +} JsonTablePlanType; + +/* + * JsonTablePlanJoinType - + * JSON_TABLE join types for JSTP_JOINED plans + */ +typedef enum JsonTablePlanJoinType +{ + JSTP_JOIN_INNER, + JSTP_JOIN_OUTER, + JSTP_JOIN_CROSS, + JSTP_JOIN_UNION, +} JsonTablePlanJoinType; I can guess the enum value meaning of JsonTablePlanJoinType, but I can't guess the meaning of "JSTP_SIMPLE" or "JSTP_JOINED". adding some comments in JsonTablePlanType would make it more clear. I think I can understand JsonTableScanNextRow. but i don't understand JsonTablePlanNextRow. maybe we can add some comments on JsonTableJoinState. +-- unspecified plan (outer, union) +select + jt.* +from + jsonb_table_test jtt, + json_table ( + jtt.js,'strict $[*]' as p + columns ( + n for ordinality, + a int path 'lax $.a' default -1 on empty, + nested path 'strict $.b[*]' as pb columns ( b int path '$' ), + nested path 'strict $.c[*]' as pc columns ( c int path '$' ) + ) + ) jt; + n | a | b | c +---+----+---+---- + 1 | 1 | | + 2 | 2 | 1 | + 2 | 2 | 2 | + 2 | 2 | 3 | + 2 | 2 | | 10 + 2 | 2 | | + 2 | 2 | | 20 + 3 | 3 | 1 | + 3 | 3 | 2 | + 4 | -1 | 1 | + 4 | -1 | 2 | +(11 rows) + +-- default plan (outer, union) +select + jt.* +from + jsonb_table_test jtt, + json_table ( + jtt.js,'strict $[*]' as p + columns ( + n for ordinality, + a int path 'lax $.a' default -1 on empty, + nested path 'strict $.b[*]' as pb columns ( b int path '$' ), + nested path 'strict $.c[*]' as pc columns ( c int path '$' ) + ) + plan default (outer, union) + ) jt; + n | a | b | c +---+----+---+---- + 1 | 1 | | + 2 | 2 | 1 | 10 + 2 | 2 | 1 | + 2 | 2 | 1 | 20 + 2 | 2 | 2 | 10 + 2 | 2 | 2 | + 2 | 2 | 2 | 20 + 2 | 2 | 3 | 10 + 2 | 2 | 3 | + 2 | 2 | 3 | 20 + 3 | 3 | | + 4 | -1 | | +(12 rows) these two query results should be the same, if i understand it correctly.