Feedback-ID: ia2694551:Fastmail
Date: Thu, 7 Dec 2023 19:41:55 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Erik Rijkers <er@xs4all.nl>, jian he <jian.universality@gmail.com>,
	Andres Freund <andres@anarazel.de>, Andrew Dunstan <andrew@dunslane.net>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: remaining sql/json patches
Message-ID: <202312071841.u2gueb5dsrbk@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="f7aagnfbdwrvaw4y"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+HiwqFKhM=6fN3Np61uYGCsOGdBKk1K1MbCQ0Q6LMNWGShQ0A@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202312071841.u2gueb5dsrbk%40alvherre.pgsql>
Precedence: bulk


--f7aagnfbdwrvaw4y
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

I noticed that JSON_TABLE uses an explicit FORMAT JSON in one of the
rules, instead of using json_format_clause_opt like everywhere else.  I
wondered why, and noticed that it's because it wants to set coltype
JTC_FORMATTED when the clause is present but JTC_REGULAR otherwise.
This seemed a little odd, but I thought to split json_format_clause_opt
in two productions, one without the empty rule (json_format_clause) and
another with it.  This is not a groundbreaking improvement, but it seems
more natural, and it helps contain the FORMAT stuff a little better.

I also noticed while at it that we can do away not only with the
json_encoding_clause_opt clause, but also with makeJsonEncoding().

The attach patch does it.  This is not derived from the patches you're
currently working on; it's more of a revise of the previous SQL/JSON
code I committed in 7081ac46ace8.

It goes before your 0003 and has a couple of easily resolved conflicts
with both 0003 and 0004; then in 0004 you have to edit the JSON_TABLE
rule that has FORMAT_LA and replace that with json_format_clause.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"El que vive para el futuro es un iluso, y el que vive para el pasado,
un imbécil" (Luis Adler, "Los tripulantes de la noche")

--f7aagnfbdwrvaw4y
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-simplify-json_format_clause-by-removing-the-opt-part.patch"

From 06e0814e06d04e5ec268e08323eba1b62a656926 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 7 Dec 2023 15:56:15 +0100
Subject: [PATCH] simplify json_format_clause by removing the 'opt' part

---
 src/backend/nodes/makefuncs.c | 21 ---------------
 src/backend/parser/gram.y     | 51 +++++++++++++++++++++++------------
 src/include/nodes/makefuncs.h |  1 -
 3 files changed, 34 insertions(+), 39 deletions(-)

diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index c6fb571982..89e77adbc7 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -857,27 +857,6 @@ makeJsonValueExpr(Expr *raw_expr, Expr *formatted_expr,
 	return jve;
 }
 
-/*
- * makeJsonEncoding -
- *	  converts JSON encoding name to enum JsonEncoding
- */
-JsonEncoding
-makeJsonEncoding(char *name)
-{
-	if (!pg_strcasecmp(name, "utf8"))
-		return JS_ENC_UTF8;
-	if (!pg_strcasecmp(name, "utf16"))
-		return JS_ENC_UTF16;
-	if (!pg_strcasecmp(name, "utf32"))
-		return JS_ENC_UTF32;
-
-	ereport(ERROR,
-			errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-			errmsg("unrecognized JSON encoding: %s", name));
-
-	return JS_ENC_DEFAULT;
-}
-
 /*
  * makeJsonKeyValue -
  *	  creates a JsonKeyValue node
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index d631ac89a9..f16bbd3cdd 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -645,7 +645,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>		hash_partbound
 %type <defelt>		hash_partbound_elem
 
-%type <node>	json_format_clause_opt
+%type <node>	json_format_clause
+				json_format_clause_opt
 				json_value_expr
 				json_returning_clause_opt
 				json_name_and_value
@@ -653,8 +654,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>	json_name_and_value_list
 				json_value_expr_list
 				json_array_aggregate_order_by_clause_opt
-%type <ival>	json_encoding_clause_opt
-				json_predicate_type_constraint
+%type <ival>	json_predicate_type_constraint
 %type <boolean>	json_key_uniqueness_constraint_opt
 				json_object_constructor_null_clause_opt
 				json_array_constructor_null_clause_opt
@@ -14962,12 +14962,11 @@ a_expr:		c_expr									{ $$ = $1; }
 			/*
 			 * Required by SQL/JSON, but there are conflicts
 			| a_expr
-				FORMAT_LA JSON json_encoding_clause_opt
+				json_format_clause
 				IS  json_predicate_type_constraint
 					json_key_uniqueness_constraint_opt		%prec IS
 				{
-					$3.location = @2;
-					$$ = makeJsonIsPredicate($1, $3, $5, $6, @1);
+					$$ = makeJsonIsPredicate($1, $2, $4, $5, @1);
 				}
 			*/
 			| a_expr IS NOT
@@ -14981,13 +14980,12 @@ a_expr:		c_expr									{ $$ = $1; }
 			/*
 			 * Required by SQL/JSON, but there are conflicts
 			| a_expr
-				FORMAT_LA JSON json_encoding_clause_opt
+				json_format_clause
 				IS NOT
 					json_predicate_type_constraint
 					json_key_uniqueness_constraint_opt		%prec IS
 				{
-					$3.location = @2;
-					$$ = makeNotExpr(makeJsonIsPredicate($1, $3, $6, $7, @1), @1);
+					$$ = makeNotExpr(makeJsonIsPredicate($1, $2, $5, $6, @1), @1);
 				}
 			*/
 			| DEFAULT
@@ -16503,10 +16501,34 @@ json_value_expr:
 			}
 		;
 
-json_format_clause_opt:
-			FORMAT_LA JSON json_encoding_clause_opt
+json_format_clause:
+			FORMAT_LA JSON ENCODING name
 				{
-					$$ = (Node *) makeJsonFormat(JS_FORMAT_JSON, $3, @1);
+					int		encoding;
+
+					if (!pg_strcasecmp($4, "utf8"))
+						encoding = JS_ENC_UTF8;
+					else if (!pg_strcasecmp($4, "utf16"))
+						encoding = JS_ENC_UTF16;
+					else if (!pg_strcasecmp($4, "utf32"))
+						encoding = JS_ENC_UTF32;
+					else
+						ereport(ERROR,
+								errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+								errmsg("unrecognized JSON encoding: %s", $4));
+
+					$$ = (Node *) makeJsonFormat(JS_FORMAT_JSON, encoding, @1);
+				}
+			| FORMAT_LA JSON
+				{
+					$$ = (Node *) makeJsonFormat(JS_FORMAT_JSON, JS_ENC_DEFAULT, @1);
+				}
+		;
+
+json_format_clause_opt:
+			json_format_clause
+				{
+					$$ = $1;
 				}
 			| /* EMPTY */
 				{
@@ -16514,11 +16536,6 @@ json_format_clause_opt:
 				}
 		;
 
-json_encoding_clause_opt:
-			ENCODING name					{ $$ = makeJsonEncoding($2); }
-			| /* EMPTY */					{ $$ = JS_ENC_DEFAULT; }
-		;
-
 json_returning_clause_opt:
 			RETURNING Typename json_format_clause_opt
 				{
diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h
index 3180703005..aca0ee54df 100644
--- a/src/include/nodes/makefuncs.h
+++ b/src/include/nodes/makefuncs.h
@@ -116,6 +116,5 @@ extern Node *makeJsonKeyValue(Node *key, Node *value);
 extern Node *makeJsonIsPredicate(Node *expr, JsonFormat *format,
 								 JsonValueType item_type, bool unique_keys,
 								 int location);
-extern JsonEncoding makeJsonEncoding(char *name);
 
 #endif							/* MAKEFUNC_H */
-- 
2.39.2


--f7aagnfbdwrvaw4y--