Re: remaining sql/json patches
jian he <jian.universality@gmail.com>
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
SQL/JSON: Various improvements to SQL/JSON query function docs
- ce416fadb4b6 17.0 landed
- 42de72fa7b80 18.0 landed
-
SQL/JSON: Fix some obsolete comments.
- 290a6d800d90 17.0 landed
- 7768b6569de9 16.4 landed
- 3a8a1f3254b2 18.0 landed
-
SQL/JSON: Fix issues with DEFAULT .. ON ERROR / EMPTY
- c0fc0751862d 17.0 landed
-
JSON_TABLE: Add support for NESTED paths and columns
- bb766cde63b4 17.0 landed
-
Fix JsonExpr deparsing to emit QUOTES and WRAPPER correctly
- f6a2529920cf 17.0 landed
-
Fix typo introduced in 6185c9737
- 2f6e78b0619a 17.0 landed
-
Add basic JSON_TABLE() functionality
- de3600452b61 17.0 landed
-
Avoid splitting errmsg string to span multiple lines
- 085e759e9da7 17.0 landed
-
Add SQL/JSON query functions
- 6185c9737cf4 17.0 landed
-
Implement various jsonpath methods
- 66ea94e8e606 17.0 cited
-
Add soft error handling to some expression nodes
- aaaf9449ec6b 17.0 landed
- 7fbc75b26ed8 17.0 landed
-
Adjust populate_record_field() to handle errors softly
- 1edb3b491bee 17.0 landed
-
Refactor code used by jsonpath executor to fetch variables
- faa2b953ba3b 17.0 landed
-
Test EXPLAIN (FORMAT JSON) ... XMLTABLE
- 752533d40fd5 17.0 landed
-
Simplify productions for FORMAT JSON [ ENCODING name ]
- d3fe6e90bab5 17.0 landed
-
Add trailing commas to enum definitions
- 611806cd726f 17.0 cited
-
doc: add missing <returnvalue> and whitespace
- e055b6be7ebb 17.0 landed
-
Add more SQL/JSON constructor functions
- 03734a7fed7d 17.0 landed
-
Rename a nonterminal used in SQL/JSON grammar
- 254ac5a7c31f 17.0 landed
-
Some refactoring to export json(b) conversion functions
- b22391a2ff7b 17.0 landed
-
Don't include CaseTestExpr in JsonValueExpr.formatted_expr
- 66a9003e2e3e 16.0 landed
- b6e1157e7d33 17.0 landed
-
Code review for commit b6e1157e7d
- 7c7412cae3ea 17.0 landed
-
Pass constructName to transformJsonValueExpr()
- 7825a1b01e40 16.0 landed
- 785480c9533d 17.0 landed
-
Unify JSON categorize type API and export for external use
- 3c152a27b063 17.0 landed
-
Make some indentation in gram.y consistent
- 5edf438eeb00 17.0 landed
- 01f1f789df56 16.0 landed
-
Allow most keywords to be used as column labels without requiring AS.
- 06a7c3154f5b 14.0 cited
-
Reduce size of backend scanner's tables.
- 7f380c59f800 13.0 cited
-
Use perfect hashing, instead of binary search, for keyword lookup.
- c64d0cd5ce24 12.0 cited
Hi.
+/*
+ * JsonTableFetchRow
+ * Prepare the next "current" tuple for upcoming GetValue calls.
+ * Returns FALSE if the row-filter expression returned no more rows.
+ */
+static bool
+JsonTableFetchRow(TableFuncScanState *state)
+{
+ JsonTableExecContext *cxt =
+ GetJsonTableExecContext(state, "JsonTableFetchRow");
+
+ if (cxt->empty)
+ return false;
+
+ return JsonTableScanNextRow(cxt->root);
+}
The declaration of struct JsonbTableRoutine, SetRowFilter field is
null. So I am confused by the above comment.
also seems the `if (cxt->empty)` part never called.
+static inline JsonTableExecContext *
+GetJsonTableExecContext(TableFuncScanState *state, const char *fname)
+{
+ JsonTableExecContext *result;
+
+ if (!IsA(state, TableFuncScanState))
+ elog(ERROR, "%s called with invalid TableFuncScanState", fname);
+ result = (JsonTableExecContext *) state->opaque;
+ if (result->magic != JSON_TABLE_EXEC_CONTEXT_MAGIC)
+ elog(ERROR, "%s called with invalid TableFuncScanState", fname);
+
+ return result;
+}
I think Assert(IsA(state, TableFuncScanState)) would be better.
+/*
+ * JsonTablePlanType -
+ * flags for JSON_TABLE plan node types representation
+ */
+typedef enum JsonTablePlanType
+{
+ JSTP_DEFAULT,
+ JSTP_SIMPLE,
+ JSTP_JOINED,
+} JsonTablePlanType;
it would be better to add some comments on it. thanks.
JsonTablePlanNextRow is quite recursive! Adding more explanation would
be helpful, thanks.
+/* Recursively reset scan and its child nodes */
+static void
+JsonTableRescanRecursive(JsonTablePlanState * state)
+{
+ if (state->type == JSON_TABLE_JOIN_STATE)
+ {
+ JsonTableJoinState *join = (JsonTableJoinState *) state;
+
+ JsonTableRescanRecursive(join->left);
+ JsonTableRescanRecursive(join->right);
+ join->advanceRight = false;
+ }
+ else
+ {
+ JsonTableScanState *scan = (JsonTableScanState *) state;
+
+ Assert(state->type == JSON_TABLE_SCAN_STATE);
+ JsonTableRescan(scan);
+ if (scan->plan.nested)
+ JsonTableRescanRecursive(scan->plan.nested);
+ }
+}
From the coverage report, I noticed the first IF branch in
JsonTableRescanRecursive never called.
+ foreach(col, columns)
+ {
+ JsonTableColumn *rawc = castNode(JsonTableColumn, lfirst(col));
+ Oid typid;
+ int32 typmod;
+ Node *colexpr;
+
+ if (rawc->name)
+ {
+ /* make sure column names are unique */
+ ListCell *colname;
+
+ foreach(colname, tf->colnames)
+ if (!strcmp((const char *) colname, rawc->name))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("column name \"%s\" is not unique",
+ rawc->name),
+ parser_errposition(pstate, rawc->location)));
this `/* make sure column names are unique */` logic part already
validated in isJsonTablePathNameDuplicate, so we don't need it?
actually isJsonTablePathNameDuplicate validates both column name and pathname.
select jt.* from jsonb_table_test jtt,
json_table (jtt.js,'strict $[*]' as p
columns (n for ordinality,
nested path 'strict $.b[*]' as pb columns ( c int path '$' ),
nested path 'strict $.b[*]' as pb columns ( s int path '$' ))
) jt;
ERROR: duplicate JSON_TABLE column name: pb
HINT: JSON_TABLE column names must be distinct from one another.
the error is not very accurate, since pb is a pathname?