From 7ee6ded28aa7873adb976096c6aac2f3fe0c30cd Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Date: Mon, 6 Nov 2023 18:09:09 +0530
Subject: [PATCH v3 1/4] Reorganise jsonpath Operators and Methods

Various jsonpath operators and methods add various keywords,
switch cases, and documentation entries in some order.  However,
they are not consistent; reorder them for better maintainability or
readability.

Jeevan Chalke, per suggestion from Peter Eisentraut.
---
 doc/src/sgml/func.sgml                | 24 +++++-----
 src/backend/utils/adt/jsonpath.c      | 82 +++++++++++++++++------------------
 src/backend/utils/adt/jsonpath_exec.c | 78 ++++++++++++++++-----------------
 src/backend/utils/adt/jsonpath_gram.y | 22 +++++-----
 src/include/utils/jsonpath.h          | 25 +++++++----
 5 files changed, 119 insertions(+), 112 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d963f0a..f18f05b 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17687,43 +17687,43 @@ strict $.**.HR
 
       <row>
        <entry role="func_table_entry"><para role="func_signature">
-        <replaceable>number</replaceable> <literal>.</literal> <literal>ceiling()</literal>
+        <replaceable>number</replaceable> <literal>.</literal> <literal>abs()</literal>
         <returnvalue><replaceable>number</replaceable></returnvalue>
        </para>
        <para>
-        Nearest integer greater than or equal to the given number
+        Absolute value of the given number
        </para>
        <para>
-        <literal>jsonb_path_query('{"h": 1.3}', '$.h.ceiling()')</literal>
-        <returnvalue>2</returnvalue>
+        <literal>jsonb_path_query('{"z": -0.3}', '$.z.abs()')</literal>
+        <returnvalue>0.3</returnvalue>
        </para></entry>
       </row>
 
       <row>
        <entry role="func_table_entry"><para role="func_signature">
-        <replaceable>number</replaceable> <literal>.</literal> <literal>floor()</literal>
+        <replaceable>number</replaceable> <literal>.</literal> <literal>ceiling()</literal>
         <returnvalue><replaceable>number</replaceable></returnvalue>
        </para>
        <para>
-        Nearest integer less than or equal to the given number
+        Nearest integer greater than or equal to the given number
        </para>
        <para>
-        <literal>jsonb_path_query('{"h": 1.7}', '$.h.floor()')</literal>
-        <returnvalue>1</returnvalue>
+        <literal>jsonb_path_query('{"h": 1.3}', '$.h.ceiling()')</literal>
+        <returnvalue>2</returnvalue>
        </para></entry>
       </row>
 
       <row>
        <entry role="func_table_entry"><para role="func_signature">
-        <replaceable>number</replaceable> <literal>.</literal> <literal>abs()</literal>
+        <replaceable>number</replaceable> <literal>.</literal> <literal>floor()</literal>
         <returnvalue><replaceable>number</replaceable></returnvalue>
        </para>
        <para>
-        Absolute value of the given number
+        Nearest integer less than or equal to the given number
        </para>
        <para>
-        <literal>jsonb_path_query('{"z": -0.3}', '$.z.abs()')</literal>
-        <returnvalue>0.3</returnvalue>
+        <literal>jsonb_path_query('{"h": 1.7}', '$.h.floor()')</literal>
+        <returnvalue>1</returnvalue>
        </para></entry>
       </row>
 
diff --git a/src/backend/utils/adt/jsonpath.c b/src/backend/utils/adt/jsonpath.c
index c5ba3b7..8ff9b56 100644
--- a/src/backend/utils/adt/jsonpath.c
+++ b/src/backend/utils/adt/jsonpath.c
@@ -439,10 +439,10 @@ flattenJsonPathParseItem(StringInfo buf, int *result, struct Node *escontext,
 			break;
 		case jpiType:
 		case jpiSize:
+		case jpiDouble:
 		case jpiAbs:
-		case jpiFloor:
 		case jpiCeiling:
-		case jpiDouble:
+		case jpiFloor:
 		case jpiKeyValue:
 			break;
 		default:
@@ -610,18 +610,6 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
 			if (printBracketes)
 				appendStringInfoChar(buf, ')');
 			break;
-		case jpiPlus:
-		case jpiMinus:
-			if (printBracketes)
-				appendStringInfoChar(buf, '(');
-			appendStringInfoChar(buf, v->type == jpiPlus ? '+' : '-');
-			jspGetArg(v, &elem);
-			printJsonPathItem(buf, &elem, false,
-							  operationPriority(elem.type) <=
-							  operationPriority(v->type));
-			if (printBracketes)
-				appendStringInfoChar(buf, ')');
-			break;
 		case jpiFilter:
 			appendStringInfoString(buf, "?(");
 			jspGetArg(v, &elem);
@@ -712,23 +700,35 @@ printJsonPathItem(StringInfo buf, JsonPathItem *v, bool inKey,
 								 v->content.anybounds.first,
 								 v->content.anybounds.last);
 			break;
+		case jpiPlus:
+		case jpiMinus:
+			if (printBracketes)
+				appendStringInfoChar(buf, '(');
+			appendStringInfoChar(buf, v->type == jpiPlus ? '+' : '-');
+			jspGetArg(v, &elem);
+			printJsonPathItem(buf, &elem, false,
+							  operationPriority(elem.type) <=
+							  operationPriority(v->type));
+			if (printBracketes)
+				appendStringInfoChar(buf, ')');
+			break;
 		case jpiType:
 			appendStringInfoString(buf, ".type()");
 			break;
 		case jpiSize:
 			appendStringInfoString(buf, ".size()");
 			break;
+		case jpiDouble:
+			appendStringInfoString(buf, ".double()");
+			break;
 		case jpiAbs:
 			appendStringInfoString(buf, ".abs()");
 			break;
-		case jpiFloor:
-			appendStringInfoString(buf, ".floor()");
-			break;
 		case jpiCeiling:
 			appendStringInfoString(buf, ".ceiling()");
 			break;
-		case jpiDouble:
-			appendStringInfoString(buf, ".double()");
+		case jpiFloor:
+			appendStringInfoString(buf, ".floor()");
 			break;
 		case jpiDatetime:
 			appendStringInfoString(buf, ".datetime(");
@@ -771,11 +771,11 @@ jspOperationName(JsonPathItemType type)
 			return "<=";
 		case jpiGreaterOrEqual:
 			return ">=";
-		case jpiPlus:
 		case jpiAdd:
+		case jpiPlus:
 			return "+";
-		case jpiMinus:
 		case jpiSub:
+		case jpiMinus:
 			return "-";
 		case jpiMul:
 			return "*";
@@ -783,26 +783,26 @@ jspOperationName(JsonPathItemType type)
 			return "/";
 		case jpiMod:
 			return "%";
-		case jpiStartsWith:
-			return "starts with";
-		case jpiLikeRegex:
-			return "like_regex";
 		case jpiType:
 			return "type";
 		case jpiSize:
 			return "size";
-		case jpiKeyValue:
-			return "keyvalue";
 		case jpiDouble:
 			return "double";
 		case jpiAbs:
 			return "abs";
-		case jpiFloor:
-			return "floor";
 		case jpiCeiling:
 			return "ceiling";
+		case jpiFloor:
+			return "floor";
 		case jpiDatetime:
 			return "datetime";
+		case jpiKeyValue:
+			return "keyvalue";
+		case jpiStartsWith:
+			return "starts with";
+		case jpiLikeRegex:
+			return "like_regex";
 		default:
 			elog(ERROR, "unrecognized jsonpath item type: %d", type);
 			return NULL;
@@ -893,10 +893,10 @@ jspInitByBuffer(JsonPathItem *v, char *base, int32 pos)
 		case jpiAnyKey:
 		case jpiType:
 		case jpiSize:
+		case jpiDouble:
 		case jpiAbs:
-		case jpiFloor:
 		case jpiCeiling:
-		case jpiDouble:
+		case jpiFloor:
 		case jpiKeyValue:
 		case jpiLast:
 			break;
@@ -935,9 +935,9 @@ jspInitByBuffer(JsonPathItem *v, char *base, int32 pos)
 		case jpiNot:
 		case jpiExists:
 		case jpiIsUnknown:
+		case jpiFilter:
 		case jpiPlus:
 		case jpiMinus:
-		case jpiFilter:
 		case jpiDatetime:
 			read_int32(v->content.arg, base, pos);
 			break;
@@ -989,13 +989,6 @@ jspGetNext(JsonPathItem *v, JsonPathItem *a)
 			   v->type == jpiRoot ||
 			   v->type == jpiVariable ||
 			   v->type == jpiLast ||
-			   v->type == jpiAdd ||
-			   v->type == jpiSub ||
-			   v->type == jpiMul ||
-			   v->type == jpiDiv ||
-			   v->type == jpiMod ||
-			   v->type == jpiPlus ||
-			   v->type == jpiMinus ||
 			   v->type == jpiEqual ||
 			   v->type == jpiNotEqual ||
 			   v->type == jpiGreater ||
@@ -1006,12 +999,19 @@ jspGetNext(JsonPathItem *v, JsonPathItem *a)
 			   v->type == jpiOr ||
 			   v->type == jpiNot ||
 			   v->type == jpiIsUnknown ||
+			   v->type == jpiAdd ||
+			   v->type == jpiPlus ||
+			   v->type == jpiSub ||
+			   v->type == jpiMinus ||
+			   v->type == jpiMul ||
+			   v->type == jpiDiv ||
+			   v->type == jpiMod ||
 			   v->type == jpiType ||
 			   v->type == jpiSize ||
+			   v->type == jpiDouble ||
 			   v->type == jpiAbs ||
-			   v->type == jpiFloor ||
 			   v->type == jpiCeiling ||
-			   v->type == jpiDouble ||
+			   v->type == jpiFloor ||
 			   v->type == jpiDatetime ||
 			   v->type == jpiKeyValue ||
 			   v->type == jpiStartsWith ||
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 2d0599b..766335e 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -874,33 +874,6 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 			}
 			break;
 
-		case jpiAdd:
-			return executeBinaryArithmExpr(cxt, jsp, jb,
-										   numeric_add_opt_error, found);
-
-		case jpiSub:
-			return executeBinaryArithmExpr(cxt, jsp, jb,
-										   numeric_sub_opt_error, found);
-
-		case jpiMul:
-			return executeBinaryArithmExpr(cxt, jsp, jb,
-										   numeric_mul_opt_error, found);
-
-		case jpiDiv:
-			return executeBinaryArithmExpr(cxt, jsp, jb,
-										   numeric_div_opt_error, found);
-
-		case jpiMod:
-			return executeBinaryArithmExpr(cxt, jsp, jb,
-										   numeric_mod_opt_error, found);
-
-		case jpiPlus:
-			return executeUnaryArithmExpr(cxt, jsp, jb, NULL, found);
-
-		case jpiMinus:
-			return executeUnaryArithmExpr(cxt, jsp, jb, numeric_uminus,
-										  found);
-
 		case jpiFilter:
 			{
 				JsonPathBool st;
@@ -980,6 +953,33 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 			}
 			break;
 
+		case jpiAdd:
+			return executeBinaryArithmExpr(cxt, jsp, jb,
+										   numeric_add_opt_error, found);
+
+		case jpiPlus:
+			return executeUnaryArithmExpr(cxt, jsp, jb, NULL, found);
+
+		case jpiSub:
+			return executeBinaryArithmExpr(cxt, jsp, jb,
+										   numeric_sub_opt_error, found);
+
+		case jpiMinus:
+			return executeUnaryArithmExpr(cxt, jsp, jb, numeric_uminus,
+										  found);
+
+		case jpiMul:
+			return executeBinaryArithmExpr(cxt, jsp, jb,
+										   numeric_mul_opt_error, found);
+
+		case jpiDiv:
+			return executeBinaryArithmExpr(cxt, jsp, jb,
+										   numeric_div_opt_error, found);
+
+		case jpiMod:
+			return executeBinaryArithmExpr(cxt, jsp, jb,
+										   numeric_mod_opt_error, found);
+
 		case jpiType:
 			{
 				JsonbValue *jbv = palloc(sizeof(*jbv));
@@ -1021,18 +1021,6 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 			}
 			break;
 
-		case jpiAbs:
-			return executeNumericItemMethod(cxt, jsp, jb, unwrap, numeric_abs,
-											found);
-
-		case jpiFloor:
-			return executeNumericItemMethod(cxt, jsp, jb, unwrap, numeric_floor,
-											found);
-
-		case jpiCeiling:
-			return executeNumericItemMethod(cxt, jsp, jb, unwrap, numeric_ceil,
-											found);
-
 		case jpiDouble:
 			{
 				JsonbValue	jbv;
@@ -1098,6 +1086,18 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 			}
 			break;
 
+		case jpiAbs:
+			return executeNumericItemMethod(cxt, jsp, jb, unwrap, numeric_abs,
+											found);
+
+		case jpiCeiling:
+			return executeNumericItemMethod(cxt, jsp, jb, unwrap, numeric_ceil,
+											found);
+
+		case jpiFloor:
+			return executeNumericItemMethod(cxt, jsp, jb, unwrap, numeric_floor,
+											found);
+
 		case jpiDatetime:
 			if (unwrap && JsonbType(jb) == jbvArray)
 				return executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
diff --git a/src/backend/utils/adt/jsonpath_gram.y b/src/backend/utils/adt/jsonpath_gram.y
index adc259d..4233eed 100644
--- a/src/backend/utils/adt/jsonpath_gram.y
+++ b/src/backend/utils/adt/jsonpath_gram.y
@@ -80,7 +80,7 @@ static bool makeItemLikeRegex(JsonPathParseItem *expr,
 %token	<str>		OR_P AND_P NOT_P
 %token	<str>		LESS_P LESSEQUAL_P EQUAL_P NOTEQUAL_P GREATEREQUAL_P GREATER_P
 %token	<str>		ANY_P STRICT_P LAX_P LAST_P STARTS_P WITH_P LIKE_REGEX_P FLAG_P
-%token	<str>		ABS_P SIZE_P TYPE_P FLOOR_P DOUBLE_P CEILING_P KEYVALUE_P
+%token	<str>		TYPE_P SIZE_P DOUBLE_P ABS_P CEILING_P FLOOR_P KEYVALUE_P
 %token	<str>		DATETIME_P
 
 %type	<result>	result
@@ -206,10 +206,10 @@ accessor_expr:
 expr:
 	accessor_expr					{ $$ = makeItemList($1); }
 	| '(' expr ')'					{ $$ = $2; }
-	| '+' expr %prec UMINUS			{ $$ = makeItemUnary(jpiPlus, $2); }
-	| '-' expr %prec UMINUS			{ $$ = makeItemUnary(jpiMinus, $2); }
 	| expr '+' expr					{ $$ = makeItemBinary(jpiAdd, $1, $3); }
+	| '+' expr %prec UMINUS			{ $$ = makeItemUnary(jpiPlus, $2); }
 	| expr '-' expr					{ $$ = makeItemBinary(jpiSub, $1, $3); }
+	| '-' expr %prec UMINUS			{ $$ = makeItemUnary(jpiMinus, $2); }
 	| expr '*' expr					{ $$ = makeItemBinary(jpiMul, $1, $3); }
 	| expr '/' expr					{ $$ = makeItemBinary(jpiDiv, $1, $3); }
 	| expr '%' expr					{ $$ = makeItemBinary(jpiMod, $1, $3); }
@@ -278,28 +278,28 @@ key_name:
 	| EXISTS_P
 	| STRICT_P
 	| LAX_P
-	| ABS_P
-	| SIZE_P
+	| LAST_P
+	| FLAG_P
 	| TYPE_P
-	| FLOOR_P
+	| SIZE_P
 	| DOUBLE_P
+	| ABS_P
 	| CEILING_P
+	| FLOOR_P
 	| DATETIME_P
 	| KEYVALUE_P
-	| LAST_P
 	| STARTS_P
 	| WITH_P
 	| LIKE_REGEX_P
-	| FLAG_P
 	;
 
 method:
-	ABS_P							{ $$ = jpiAbs; }
+	TYPE_P							{ $$ = jpiType; }
 	| SIZE_P						{ $$ = jpiSize; }
-	| TYPE_P						{ $$ = jpiType; }
-	| FLOOR_P						{ $$ = jpiFloor; }
 	| DOUBLE_P						{ $$ = jpiDouble; }
+	| ABS_P							{ $$ = jpiAbs; }
 	| CEILING_P						{ $$ = jpiCeiling; }
+	| FLOOR_P						{ $$ = jpiFloor; }
 	| KEYVALUE_P					{ $$ = jpiKeyValue; }
 	;
 %%
diff --git a/src/include/utils/jsonpath.h b/src/include/utils/jsonpath.h
index f0181e0..59dc233 100644
--- a/src/include/utils/jsonpath.h
+++ b/src/include/utils/jsonpath.h
@@ -66,13 +66,6 @@ typedef enum JsonPathItemType
 	jpiGreater,					/* expr > expr */
 	jpiLessOrEqual,				/* expr <= expr */
 	jpiGreaterOrEqual,			/* expr >= expr */
-	jpiAdd,						/* expr + expr */
-	jpiSub,						/* expr - expr */
-	jpiMul,						/* expr * expr */
-	jpiDiv,						/* expr / expr */
-	jpiMod,						/* expr % expr */
-	jpiPlus,					/* + expr */
-	jpiMinus,					/* - expr */
 	jpiAnyArray,				/* [*] */
 	jpiAnyKey,					/* .* */
 	jpiIndexArray,				/* [subscript, ...] */
@@ -83,14 +76,28 @@ typedef enum JsonPathItemType
 	jpiVariable,				/* $variable */
 	jpiFilter,					/* ? (predicate) */
 	jpiExists,					/* EXISTS (expr) predicate */
+
+	/*
+	 * For better maintainability or readability, keep the order of the below
+	 * jsonpath Operators and Methods at the other places, like in the
+	 * documentation, switch() cases, keywords list, etc., too.
+	 */
+	jpiAdd,						/* expr + expr */
+	jpiPlus,					/* + expr */
+	jpiSub,						/* expr - expr */
+	jpiMinus,					/* - expr */
+	jpiMul,						/* expr * expr */
+	jpiDiv,						/* expr / expr */
+	jpiMod,						/* expr % expr */
 	jpiType,					/* .type() item method */
 	jpiSize,					/* .size() item method */
+	jpiDouble,					/* .double() item method */
 	jpiAbs,						/* .abs() item method */
-	jpiFloor,					/* .floor() item method */
 	jpiCeiling,					/* .ceiling() item method */
-	jpiDouble,					/* .double() item method */
+	jpiFloor,					/* .floor() item method */
 	jpiDatetime,				/* .datetime() item method */
 	jpiKeyValue,				/* .keyvalue() item method */
+
 	jpiSubscript,				/* array subscript: 'expr' or 'expr TO expr' */
 	jpiLast,					/* LAST array subscript */
 	jpiStartsWith,				/* STARTS WITH predicate */
-- 
1.8.3.1

