Subject: Missing quotes when deparsing XMLTABLE() and SQL/JSON query functions

From: Dean Rasheed <[email-masked]>
Date: Sat, 11 Jan 2025 12:30:38 +0000

While looking over ruleutils.c, I noticed a couple of places that fail
to properly quote certain outputs:

1. XML namespace names in XMLTABLE().

2. variable names in the PASSING clause of the SQL/JSON query
functions JSON_EXISTS(), JSON_QUERY(), and JSON_VALUE() (but not
JSON_TABLE(), which has similar code that does quote variable names
properly).

I scanned the rest of ruleutils.c, and didn't find any other issues.

Regards,
Dean
From 868ea003a54adeeae470407321fe4481c95dfccb Mon Sep 17 00:00:00 2001
From: Dean Rasheed <[email-masked]>
Date: Sat, 11 Jan 2025 11:08:50 +0000
Subject: [PATCH v1 1/2] Fix XMLTABLE() deparsing to quote namespace names if
 necessary.

When deparsing an XMLTABLE() expression, XML namespace names were not
quoted. However, since they are parsed as ColLabel tokens, some names
require double quotes to ensure that they are properly interpreted.
Fix by using quote_identifier() in the deparsing code.

Back-patch to all supported versions.
---
 src/backend/utils/adt/ruleutils.c |  3 ++-
 src/test/regress/expected/xml.out | 10 +++++++---
 src/test/regress/sql/xml.sql      |  8 +++++---
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 16d15f9efb..b935868716 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -11814,7 +11814,8 @@ get_xmltable(TableFunc *tf, deparse_context *context, bool showimplicit)
 			if (ns_node != NULL)
 			{
 				get_rule_expr(expr, context, showimplicit);
-				appendStringInfo(buf, " AS %s", strVal(ns_node));
+				appendStringInfo(buf, " AS %s",
+								 quote_identifier(strVal(ns_node)));
 			}
 			else
 			{
diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index fb5f345855..2e9616acda 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -1379,16 +1379,20 @@ SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x.y' AS zz),
  10
 (1 row)
 
-CREATE VIEW xmltableview2 AS SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x.y' AS zz),
-                      '/zz:rows/zz:row'
+CREATE VIEW xmltableview2 AS SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x.y' AS "Zz"),
+                      '/Zz:rows/Zz:row'
                       PASSING '<rows xmlns="http://x.y"><row><a>10</a></row></rows>'
-                      COLUMNS a int PATH 'zz:a');
+                      COLUMNS a int PATH 'Zz:a');
 SELECT * FROM xmltableview2;
  a  
 ----
  10
 (1 row)
 
+\sv xmltableview2
+CREATE OR REPLACE VIEW public.xmltableview2 AS
+ SELECT a
+   FROM XMLTABLE(XMLNAMESPACES ('http://x.y'::text AS "Zz"), ('/Zz:rows/Zz:row'::text) PASSING ('<rows xmlns="http://x.y"><row><a>10</a></row></rows>'::xml) COLUMNS a integer PATH ('Zz:a'::text))
 SELECT * FROM XMLTABLE(XMLNAMESPACES(DEFAULT 'http://x.y'),
                       '/rows/row'
                       PASSING '<rows xmlns="http://x.y"><row><a>10</a></row></rows>'
diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index f752ecb142..bac0388ac1 100644
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -439,13 +439,15 @@ SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x.y' AS zz),
                       PASSING '<rows xmlns="http://x.y"><row><a>10</a></row></rows>'
                       COLUMNS a int PATH 'zz:a');
 
-CREATE VIEW xmltableview2 AS SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x.y' AS zz),
-                      '/zz:rows/zz:row'
+CREATE VIEW xmltableview2 AS SELECT * FROM XMLTABLE(XMLNAMESPACES('http://x.y' AS "Zz"),
+                      '/Zz:rows/Zz:row'
                       PASSING '<rows xmlns="http://x.y"><row><a>10</a></row></rows>'
-                      COLUMNS a int PATH 'zz:a');
+                      COLUMNS a int PATH 'Zz:a');
 
 SELECT * FROM xmltableview2;
 
+\sv xmltableview2
+
 SELECT * FROM XMLTABLE(XMLNAMESPACES(DEFAULT 'http://x.y'),
                       '/rows/row'
                       PASSING '<rows xmlns="http://x.y"><row><a>10</a></row></rows>'
-- 
2.43.0

From 260d4504cf5648c9034cdf2efa3faa4436eef299 Mon Sep 17 00:00:00 2001
From: Dean Rasheed <[email-masked]>
Date: Sat, 11 Jan 2025 11:34:32 +0000
Subject: [PATCH v1 2/2] Fix JsonExpr deparsing to quote variable names in the
 PASSING clause.

When deparsing a JsonExpr, variable names in the PASSING clause were
not quoted. However, since they are parsed as ColLabel tokens, some
variable names require double quotes to ensure that they are properly
interpreted. Fix by using quote_identifier() in the deparsing code.

This oversight was limited to the SQL/JSON query functions
JSON_EXISTS(), JSON_QUERY(), and JSON_VALUE().

Back-patch to v17, where these functions were added.
---
 src/backend/utils/adt/ruleutils.c             |  2 +-
 .../regress/expected/sqljson_queryfuncs.out   | 20 ++++++++++++++++---
 src/test/regress/sql/sqljson_queryfuncs.sql   |  5 ++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b935868716..2a77f715fb 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10443,7 +10443,7 @@ get_rule_expr(Node *node, deparse_context *context,
 
 						get_rule_expr((Node *) lfirst(lc2), context, showimplicit);
 						appendStringInfo(buf, " AS %s",
-										 ((String *) lfirst_node(String, lc1))->sval);
+										 quote_identifier(lfirst_node(String, lc1)->sval));
 					}
 				}
 
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 175349f7dc..329ef67496 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -1084,7 +1084,7 @@ CREATE TABLE test_jsonb_constraints (
 	CONSTRAINT test_jsonb_constraint1
 		CHECK (js IS JSON)
 	CONSTRAINT test_jsonb_constraint2
-		CHECK (JSON_EXISTS(js::jsonb, '$.a' PASSING i + 5 AS int, i::text AS txt, array[1,2,3] as arr))
+		CHECK (JSON_EXISTS(js::jsonb, '$.a' PASSING i + 5 AS int, i::text AS "TXT", array[1,2,3] as arr))
 	CONSTRAINT test_jsonb_constraint3
 		CHECK (JSON_VALUE(js::jsonb, '$.a' RETURNING int DEFAULT '12' ON EMPTY ERROR ON ERROR) > i)
 	CONSTRAINT test_jsonb_constraint4
@@ -1101,7 +1101,7 @@ CREATE TABLE test_jsonb_constraints (
  x      | jsonb   |           |          | JSON_QUERY('[1, 2]'::jsonb, '$[*]' RETURNING jsonb WITH UNCONDITIONAL WRAPPER KEEP QUOTES)
 Check constraints:
     "test_jsonb_constraint1" CHECK (js IS JSON)
-    "test_jsonb_constraint2" CHECK (JSON_EXISTS(js::jsonb, '$."a"' PASSING i + 5 AS int, i::text AS txt, ARRAY[1, 2, 3] AS arr))
+    "test_jsonb_constraint2" CHECK (JSON_EXISTS(js::jsonb, '$."a"' PASSING i + 5 AS "int", i::text AS "TXT", ARRAY[1, 2, 3] AS arr))
     "test_jsonb_constraint3" CHECK (JSON_VALUE(js::jsonb, '$."a"' RETURNING integer DEFAULT 12 ON EMPTY ERROR ON ERROR) > i)
     "test_jsonb_constraint4" CHECK (JSON_QUERY(js::jsonb, '$."a"' RETURNING jsonb WITH CONDITIONAL WRAPPER KEEP QUOTES EMPTY OBJECT ON ERROR) = '[10]'::jsonb)
     "test_jsonb_constraint5" CHECK (JSON_QUERY(js::jsonb, '$."a"' RETURNING character(5) WITHOUT WRAPPER OMIT QUOTES EMPTY ARRAY ON EMPTY) > ('a'::bpchar COLLATE "C"))
@@ -1116,7 +1116,7 @@ ORDER BY 1;
  (JSON_QUERY((js)::jsonb, '$."a"' RETURNING jsonb WITH CONDITIONAL WRAPPER KEEP QUOTES EMPTY OBJECT ON ERROR) = '[10]'::jsonb)
  (JSON_VALUE((js)::jsonb, '$."a"' RETURNING integer DEFAULT 12 ON EMPTY ERROR ON ERROR) > i)
  (js IS JSON)
- JSON_EXISTS((js)::jsonb, '$."a"' PASSING (i + 5) AS int, (i)::text AS txt, ARRAY[1, 2, 3] AS arr)
+ JSON_EXISTS((js)::jsonb, '$."a"' PASSING (i + 5) AS "int", (i)::text AS "TXT", ARRAY[1, 2, 3] AS arr)
 (5 rows)
 
 SELECT pg_get_expr(adbin, adrelid)
@@ -1366,6 +1366,20 @@ SELECT JSON_QUERY(jsonb 'null', '$xyz' PASSING 1 AS xyz);
  1
 (1 row)
 
+SELECT JSON_QUERY(jsonb 'null', '$Xyz' PASSING 1 AS Xyz);
+ERROR:  could not find jsonpath variable "Xyz"
+SELECT JSON_QUERY(jsonb 'null', '$Xyz' PASSING 1 AS "Xyz");
+ json_query 
+------------
+ 1
+(1 row)
+
+SELECT JSON_QUERY(jsonb 'null', '$"Xyz"' PASSING 1 AS "Xyz");
+ json_query 
+------------
+ 1
+(1 row)
+
 -- Test ON ERROR / EMPTY value validity for the function; all fail.
 SELECT JSON_EXISTS(jsonb '1', '$' DEFAULT 1 ON ERROR);
 ERROR:  invalid ON ERROR behavior
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index 21b5d49ece..8d7b225b61 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -327,7 +327,7 @@ CREATE TABLE test_jsonb_constraints (
 	CONSTRAINT test_jsonb_constraint1
 		CHECK (js IS JSON)
 	CONSTRAINT test_jsonb_constraint2
-		CHECK (JSON_EXISTS(js::jsonb, '$.a' PASSING i + 5 AS int, i::text AS txt, array[1,2,3] as arr))
+		CHECK (JSON_EXISTS(js::jsonb, '$.a' PASSING i + 5 AS int, i::text AS "TXT", array[1,2,3] as arr))
 	CONSTRAINT test_jsonb_constraint3
 		CHECK (JSON_VALUE(js::jsonb, '$.a' RETURNING int DEFAULT '12' ON EMPTY ERROR ON ERROR) > i)
 	CONSTRAINT test_jsonb_constraint4
@@ -465,6 +465,9 @@ SELECT json_value('"aaa"', path RETURNING json) FROM jsonpaths;
 SELECT JSON_QUERY(jsonb 'null', '$xyz' PASSING 1 AS xy);
 SELECT JSON_QUERY(jsonb 'null', '$xy' PASSING 1 AS xyz);
 SELECT JSON_QUERY(jsonb 'null', '$xyz' PASSING 1 AS xyz);
+SELECT JSON_QUERY(jsonb 'null', '$Xyz' PASSING 1 AS Xyz);
+SELECT JSON_QUERY(jsonb 'null', '$Xyz' PASSING 1 AS "Xyz");
+SELECT JSON_QUERY(jsonb 'null', '$"Xyz"' PASSING 1 AS "Xyz");
 
 -- Test ON ERROR / EMPTY value validity for the function; all fail.
 SELECT JSON_EXISTS(jsonb '1', '$' DEFAULT 1 ON ERROR);
-- 
2.43.0