Attached are two small fixup patches for your patch set.
In the first one, I simplified the grammar for the .decimal() method.
It seemed a bit overkill to build a whole list structure when all we
need are 0, 1, or 2 arguments.
Per SQL standard, the precision and scale arguments are unsigned
integers, so unary plus and minus signs are not supported. So my patch
removes that support, but I didn't adjust the regression tests for that.
Also note that in your 0002 patch, the datetime precision is similarly
unsigned, so that's consistent.
By the way, in your 0002 patch, don't see the need for the separate
datetime_method grammar rule. You can fold that into accessor_op.
Overall, I think it would be better if you combined all three of these
patches into one. Right now, you have arranged these as incremental
features, and as a result of that, the additions to the JsonPathItemType
enum and the grammar keywords etc. are ordered in the way you worked on
these features, I guess. It would be good to maintain a bit of sanity
to put all of this together and order all the enums and everything else
for example in the order they are in the sql_features.txt file (which is
alphabetical, I suppose). At this point I suspect we'll end up
committing this whole feature set together anyway, so we might as well
organize it that way.
From 81e330a243d85dff7f64adf17815258e2764ea01 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 14:11:36 +0100
Subject: [PATCH 1/2] fixup! Implement jsonpath .number(), .decimal([precision
[, scale]]), .bigint(), and .integer() methods
---
src/backend/utils/adt/jsonpath_gram.y | 43 +++++----------------------
1 file changed, 8 insertions(+), 35 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_gram.y
b/src/backend/utils/adt/jsonpath_gram.y
index 24c31047ffd..9c06c3f6cb9 100644
--- a/src/backend/utils/adt/jsonpath_gram.y
+++ b/src/backend/utils/adt/jsonpath_gram.y
@@ -89,9 +89,9 @@ static bool makeItemLikeRegex(JsonPathParseItem *expr,
%type <value> scalar_value path_primary expr array_accessor
any_path accessor_op key predicate
delimited_predicate
index_elem starts_with_initial
expr_or_predicate
- datetime_template opt_datetime_template
csv_elem
+ datetime_template opt_datetime_template
-%type <elems> accessor_expr csv_list opt_csv_list
+%type <elems> accessor_expr
%type <indexs> index_list
@@ -252,39 +252,12 @@ accessor_op:
| '.' DATETIME_P '(' opt_datetime_template ')'
{ $$ =
makeItemUnary(jpiDatetime, $4); }
| '?' '(' predicate ')' { $$ = makeItemUnary(jpiFilter,
$3); }
- | '.' DECIMAL_P '(' opt_csv_list ')'
- {
- if (list_length($4) == 0)
- $$ = makeItemBinary(jpiDecimal, NULL, NULL);
- else if (list_length($4) == 1)
- $$ = makeItemBinary(jpiDecimal, linitial($4), NULL);
- else if (list_length($4) == 2)
- $$ = makeItemBinary(jpiDecimal, linitial($4),
lsecond($4));
- else
- ereturn(escontext, false,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("invalid input syntax for type
%s", "jsonpath"),
- errdetail(".decimal() can only have an
optional precision[,scale].")));
- }
- ;
-
-csv_elem:
- INT_P
- { $$ = makeItemNumeric(&$1); }
- | '+' INT_P %prec UMINUS
- { $$ = makeItemUnary(jpiPlus, makeItemNumeric(&$2)); }
- | '-' INT_P %prec UMINUS
- { $$ = makeItemUnary(jpiMinus, makeItemNumeric(&$2)); }
- ;
-
-csv_list:
- csv_elem { $$ =
list_make1($1); }
- | csv_list ',' csv_elem { $$ = lappend($1, $3); }
- ;
-
-opt_csv_list:
- csv_list { $$ = $1; }
- | /* EMPTY */ { $$ = NULL; }
+ | '.' DECIMAL_P '(' ')'
+ { $$ = makeItemBinary(jpiDecimal, NULL, NULL); }
+ | '.' DECIMAL_P '(' INT_P ')'
+ { $$ = makeItemBinary(jpiDecimal, makeItemNumeric(&$4), NULL); }
+ | '.' DECIMAL_P '(' INT_P ',' INT_P ')'
+ { $$ = makeItemBinary(jpiDecimal, makeItemNumeric(&$4),
makeItemNumeric(&$6)); }
;
datetime_template:
--
2.43.0
From f2f9176548d36d3b02bd3baf1d5afdf0a84a84b1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 14:19:22 +0100
Subject: [PATCH 2/2] fixup! Implement jsonpath .number(), .decimal([precision
[, scale]]), .bigint(), and .integer() methods
---
src/backend/utils/adt/jsonpath_exec.c | 48 +++++++++------------------
1 file changed, 16 insertions(+), 32 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c
b/src/backend/utils/adt/jsonpath_exec.c
index a361c456c54..092bcda45ee 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -1118,8 +1118,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
char *numstr = NULL;
if (unwrap && JsonbType(jb) == jbvArray)
- return
executeItemUnwrapTargetArray(cxt, jsp, jb, found,
-
false);
+ return
executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
if (jb->type == jbvNumeric)
{
@@ -1131,8 +1130,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
jspOperationName(jsp->type)))));
if (jsp->type == jpiDecimal)
- numstr =
DatumGetCString(DirectFunctionCall1(numeric_out,
-
NumericGetDatum(num)));
+ numstr =
DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(num)));
res = jperOk;
}
else if (jb->type == jbvString)
@@ -1144,10 +1142,8 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
numstr = pnstrdup(jb->val.string.val,
jb->val.string.len);
- noerr =
DirectInputFunctionCallSafe(numeric_in, numstr,
-
InvalidOid, -1,
-
(Node *) &escontext,
-
&datum);
+ noerr =
DirectInputFunctionCallSafe(numeric_in, numstr, InvalidOid, -1,
+
(Node *) &escontext, &datum);
if (!noerr || escontext.error_occurred)
RETURN_ERROR(ereport(ERROR,
@@ -1195,8 +1191,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
if (elem.type != jpiNumeric)
elog(ERROR, "invalid jsonpath
item type for .decimal() precision");
- precision =
numeric_int4_opt_error(jspGetNumeric(&elem),
-
&have_error);
+ precision =
numeric_int4_opt_error(jspGetNumeric(&elem), &have_error);
if (have_error)
RETURN_ERROR(ereport(ERROR,
(errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
@@ -1209,8 +1204,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
if (elem.type != jpiNumeric)
elog(ERROR, "invalid
jsonpath item type for .decimal() scale");
- scale =
numeric_int4_opt_error(jspGetNumeric(&elem),
-
&have_error);
+ scale =
numeric_int4_opt_error(jspGetNumeric(&elem), &have_error);
if (have_error)
RETURN_ERROR(ereport(ERROR,
(errcode(ERRCODE_NON_NUMERIC_SQL_JSON_ITEM),
@@ -1228,8 +1222,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
datums[1] = CStringGetDatum(sstr);
arrtypmod =
construct_array_builtin(datums, 2, CSTRINGOID);
- dtypmod =
DirectFunctionCall1(numerictypmodin,
-
PointerGetDatum(arrtypmod));
+ dtypmod =
DirectFunctionCall1(numerictypmodin, PointerGetDatum(arrtypmod));
/* Convert numstr to Numeric with
typmod */
Assert(numstr != NULL);
@@ -1262,8 +1255,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
Datum datum;
if (unwrap && JsonbType(jb) == jbvArray)
- return
executeItemUnwrapTargetArray(cxt, jsp, jb, found,
-
false);
+ return
executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
if (jb->type == jbvNumeric)
{
@@ -1288,10 +1280,8 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
ErrorSaveContext escontext =
{T_ErrorSaveContext};
bool noerr;
- noerr =
DirectInputFunctionCallSafe(int8in, tmp,
-
InvalidOid, -1,
-
(Node *) &escontext,
-
&datum);
+ noerr =
DirectInputFunctionCallSafe(int8in, tmp, InvalidOid, -1,
+
(Node *) &escontext, &datum);
if (!noerr || escontext.error_occurred)
RETURN_ERROR(ereport(ERROR,
@@ -1309,8 +1299,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
jb = &jbv;
jb->type = jbvNumeric;
- jb->val.numeric =
DatumGetNumeric(DirectFunctionCall1(int8_numeric,
-
datum));
+ jb->val.numeric =
DatumGetNumeric(DirectFunctionCall1(int8_numeric, datum));
res = executeNextItem(cxt, jsp, NULL, jb,
found, true);
}
@@ -1322,8 +1311,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
Datum datum;
if (unwrap && JsonbType(jb) == jbvArray)
- return
executeItemUnwrapTargetArray(cxt, jsp, jb, found,
-
false);
+ return
executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
if (jb->type == jbvNumeric)
{
@@ -1343,15 +1331,12 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
else if (jb->type == jbvString)
{
/* cast string as integer */
- char *tmp =
pnstrdup(jb->val.string.val,
-
jb->val.string.len);
+ char *tmp =
pnstrdup(jb->val.string.val, jb->val.string.len);
ErrorSaveContext escontext =
{T_ErrorSaveContext};
bool noerr;
- noerr =
DirectInputFunctionCallSafe(int4in, tmp,
-
InvalidOid, -1,
-
(Node *) &escontext,
-
&datum);
+ noerr =
DirectInputFunctionCallSafe(int4in, tmp, InvalidOid, -1,
+
(Node *) &escontext, &datum);
if (!noerr || escontext.error_occurred)
RETURN_ERROR(ereport(ERROR,
@@ -1369,8 +1354,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt,
JsonPathItem *jsp,
jb = &jbv;
jb->type = jbvNumeric;
- jb->val.numeric =
DatumGetNumeric(DirectFunctionCall1(int4_numeric,
-
datum));
+ jb->val.numeric =
DatumGetNumeric(DirectFunctionCall1(int4_numeric, datum));
res = executeNextItem(cxt, jsp, NULL, jb,
found, true);
}
--
2.43.0
