public inbox for [email protected]  
help / color / mirror / Atom feed
From: Jeevan Chalke <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: More new SQL/JSON item methods
Date: Wed, 1 Nov 2023 12:30:02 +0530
Message-ID: <CAM2+6=UCSp74devvHYpMx5NvpyWbCO3gYvgpEYHtNYqF2j6Z9Q@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAM2+6=XjTyqrrqHAOj80r0wVQxJSxc0iyib9bPC55uFO9VKatg@mail.gmail.com>
	<[email protected]>
	<CAM2+6=XTW1COd+56ekjP=CUceZyrgyrcBge_z0ZRkCgVwcGHQg@mail.gmail.com>
	<[email protected]>

Hello,

On Tue, Oct 24, 2023 at 6:41 PM Andrew Dunstan <[email protected]> wrote:

>
> On 2023-10-19 Th 02:06, Jeevan Chalke wrote:
>
> Thanks, Peter for the comments.
>
> On Fri, Oct 6, 2023 at 5:13 PM Peter Eisentraut <[email protected]>
> wrote:
>
>> On 29.08.23 09:05, Jeevan Chalke wrote:
>> > v1-0001-Implement-jsonpath-.bigint-.integer-and-.number-m.patch
>> >
>> > This commit implements jsonpath .bigint(), .integer(), and .number()
>> > methods.  The JSON string or a numeric value is converted to the
>> > bigint, int4, and numeric type representation.
>>
>> A comment that applies to all of these: These add various keywords,
>> switch cases, documentation entries in some order.  Are we happy with
>> that?  Should we try to reorder all of that for better maintainability
>> or readability?
>>
>
> Yeah, that's the better suggestion. While implementing these methods, I
> was confused about where to put them exactly and tried keeping them in some
> logical place.
> I think once these methods get in, we can have a follow-up patch
> reorganizing all of these.
>
>
> I think it would be better to organize things how we want them before
> adding in more stuff.
>

I have tried reordering all the jsonpath Operators and Methods
consistently. With this patch, they all appear in the same order when
together in the group.

In some switch cases, they are still divided, like in
flattenJsonPathParseItem(), where 2-arg, 1-arg, and no-arg cases are
clubbed together. But I have tried to keep them in order in those subgroups.

I will rebase my patches for this task on this patch, but before doing so,
I  would like to get your views on this reordering.

Thanks


>
> cheers
>
>
> andrew
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
>

-- 
Jeevan Chalke

*Senior Staff SDE, Database Architect, and ManagerProduct Development*



edbpostgres.com


Attachments:

  [application/octet-stream] reorder-jsonpath-Operators-Methods.patch (14.5K, ../CAM2+6=UCSp74devvHYpMx5NvpyWbCO3gYvgpEYHtNYqF2j6Z9Q@mail.gmail.com/3-reorder-jsonpath-Operators-Methods.patch)
  download | inline diff:
commit b5146b327385e50bbb10f509d9d6b59f4b60755c
Author: Jeevan Chalke <[email protected]>
Date:   Tue Oct 31 20:41:30 2023 +0530

    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.

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c76ec52..4f319c1 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17641,43 +17641,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..871d05b 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,12 +76,19 @@ typedef enum JsonPathItemType
 	jpiVariable,				/* $variable */
 	jpiFilter,					/* ? (predicate) */
 	jpiExists,					/* EXISTS (expr) predicate */
+	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' */


view thread (11+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: More new SQL/JSON item methods
  In-Reply-To: <CAM2+6=UCSp74devvHYpMx5NvpyWbCO3gYvgpEYHtNYqF2j6Z9Q@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox