Make JSON_TABLE generated path names avoid collisions

generateJsonTablePathName() produced names of the form
"json_table_path_N" in the same namespace as user-supplied path and
column names, without checking whether the name was already in use.
When an unnamed NESTED path's generated name happened to match a
user-supplied path name that a PLAN clause referenced, two sibling paths
matched the same plan entry and one of them, together with its columns,
was silently dropped from the output.

Bump the counter until the generated name is unused, so a generated name
can no longer coincide with a user-supplied one.  The still-uncovered
path is then correctly reported as not found in the plan.

The row pattern (root) path is named before the user-supplied column and
path names are collected, so when it is left unnamed its generated name
could not avoid them either, and a user column or path named like a
generated name, e.g.

    SELECT * FROM JSON_TABLE(jsonb '1', '$'
                             COLUMNS (json_table_path_0 int PATH '$')) jt;

was rejected with a bogus "duplicate JSON_TABLE column or path name"
error.  Collect the user-supplied names first and generate the row
pattern path's name afterwards, so that it avoids all of them.  An
explicit root path name is still seeded into the namespace, so a column
duplicating it is still correctly rejected.

Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv7aZGSExnbjJRw8eKkoXbu34TdoKLLA2gPye3aHjO5OSA@mail.gmail.com
Discussion: https://postgr.es/m/CAA-aLv5U94KD4C%2BLhAPYcCeGvs1xBMngcS5oEkZHN9YWwXUHsA%40mail.gmail.com

diff --git a/src/backend/parser/parse_jsontable.c b/src/backend/parser/parse_jsontable.c
index 674ab37c45a841612e1074df99001cea552e19db..3fb6a511230bbe37adb2f0e27a2fb6f35dae9ae4 100644 (file)
--- a/src/backend/parser/parse_jsontable.c
+++ b/src/backend/parser/parse_jsontable.c
@@ -106,19 +106,27 @@ transformJsonTable(ParseState *pstate, JsonTable *jt)
 
        cxt.pathNameId = 0;
 
+       /*
+        * Collect the user-supplied path and column names, checking that they are
+        * distinct.  If the row pattern path has an explicit name, it shares this
+        * namespace, so seed the list with it.
+        */
+       if (rootPathSpec->name != NULL)
+               cxt.pathNames = list_make1(rootPathSpec->name);
+       CheckDuplicateColumnOrPathNames(&cxt, jt->columns);
+
        /*
         * Generate a name for the row pattern path if it was not given one.  Path
         * names are optional for every path, including when a PLAN clause is
         * present; a specific PLAN() can only reference named paths, so an
         * unnamed path that the plan must mention is caught later as a path name
-        * mismatch or a path not covered by the plan.
+        * mismatch or a path not covered by the plan.  We generate the name only
+        * after collecting the user-supplied names above, so that it cannot
+        * collide with any of them.
         */
        if (rootPathSpec->name == NULL)
                rootPathSpec->name = generateJsonTablePathName(&cxt);
 
-       cxt.pathNames = list_make1(rootPathSpec->name);
-       CheckDuplicateColumnOrPathNames(&cxt, jt->columns);
-
        /*
         * We make lateral_only names of this level visible, whether or not the
         * RangeTableFunc is explicitly marked LATERAL.  This is needed for SQL
@@ -249,12 +257,21 @@ static char *
 generateJsonTablePathName(JsonTableParseContext *cxt)
 {
        char            namebuf[32];
-       char       *name = namebuf;
+       char       *name;
 
-       snprintf(namebuf, sizeof(namebuf), "json_table_path_%d",
-                        cxt->pathNameId++);
+       /*
+        * Bump the counter until we produce a name that is not already used as a
+        * path or column name.  Otherwise a generated name could coincide with a
+        * user-supplied one, which would confuse PLAN clause matching and could
+        * silently drop a column.
+        */
+       do
+       {
+               snprintf(namebuf, sizeof(namebuf), "json_table_path_%d",
+                                cxt->pathNameId++);
+       } while (LookupPathOrColumnName(cxt, namebuf));
 
-       name = pstrdup(name);
+       name = pstrdup(namebuf);
        cxt->pathNames = lappend(cxt->pathNames, name);
 
        return name;

diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index 56e5a0524b23c84cc4794002897587e3e27433cd..ae64dbed303755890b097f137a7ae0dc82e0f53d 100644 (file)
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -1066,6 +1066,31 @@ ERROR:  invalid JSON_TABLE plan clause
 LINE 12:        PLAN ((p1 INNER (p12 CROSS p11)) CROSS (p2 INNER p21)...
                       ^
 DETAIL:  Expected INNER or OUTER.
+-- An unnamed NESTED path whose generated name would clash with a
+-- user-supplied path name must not silently drop columns: the generated
+-- name is made unique, so the still-uncovered path is reported instead.
+SELECT * FROM JSON_TABLE(
+       jsonb '[{"x":[1],"y":[2]}]', '$[*]' AS p0
+       COLUMNS (
+               NESTED PATH '$.x[*]' AS json_table_path_0 COLUMNS (x int PATH '$'),
+               NESTED PATH '$.y[*]' COLUMNS (y int PATH '$')
+       )
+       PLAN (p0 OUTER json_table_path_0)
+) jt;
+ERROR:  invalid JSON_TABLE specification
+LINE 5:                NESTED PATH '$.y[*]' COLUMNS (y int PATH '$')
+                       ^
+DETAIL:  PLAN clause for nested path json_table_path_1 was not found.
+-- A user-supplied name matching the pattern of generated path names must not
+-- trigger a spurious duplicate-name error: the unnamed row pattern path is
+-- named only after the user-supplied names are collected, so its generated
+-- name avoids them.
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (json_table_path_0 int PATH '$')) jt;
+ json_table_path_0 
+-------------------
+                 1
+(1 row)
+
 -- JSON_TABLE: plan execution
 CREATE TEMP TABLE jsonb_table_test (js jsonb);
 INSERT INTO jsonb_table_test

diff --git a/src/test/regress/sql/sqljson_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql
index 9cd4c5e7e1e101b178910c9945b1f1f89ac8ecf2..2a33aaec57fe77a96874af8980fed68b031a4dac 100644 (file)
--- a/src/test/regress/sql/sqljson_jsontable.sql
+++ b/src/test/regress/sql/sqljson_jsontable.sql
@@ -587,6 +587,24 @@ SELECT * FROM JSON_TABLE(
        PLAN ((p1 INNER (p12 CROSS p11)) CROSS (p2 INNER p21))
 ) jt;
 
+-- An unnamed NESTED path whose generated name would clash with a
+-- user-supplied path name must not silently drop columns: the generated
+-- name is made unique, so the still-uncovered path is reported instead.
+SELECT * FROM JSON_TABLE(
+       jsonb '[{"x":[1],"y":[2]}]', '$[*]' AS p0
+       COLUMNS (
+               NESTED PATH '$.x[*]' AS json_table_path_0 COLUMNS (x int PATH '$'),
+               NESTED PATH '$.y[*]' COLUMNS (y int PATH '$')
+       )
+       PLAN (p0 OUTER json_table_path_0)
+) jt;
+
+-- A user-supplied name matching the pattern of generated path names must not
+-- trigger a spurious duplicate-name error: the unnamed row pattern path is
+-- named only after the user-supplied names are collected, so its generated
+-- name avoids them.
+SELECT * FROM JSON_TABLE(jsonb '1', '$' COLUMNS (json_table_path_0 int PATH '$')) jt;
+
 -- JSON_TABLE: plan execution
 
 CREATE TEMP TABLE jsonb_table_test (js jsonb);