On 03.11.2017 00:32, Piotr Stefaniak wrote:

On 2017-02-28 20:08, Oleg Bartunov wrote:
The standard describes SQL/JSON path language, which used by SQL/JSON query
operators to query JSON. It defines path language as string literal. We
implemented the path language as  JSONPATH data type, since other
approaches are not friendly to planner and executor.
I was a bit sad to discover that I can't
PREPARE jsq AS SELECT JSON_QUERY('{}', $1);
I assume because of this part of the updated grammar:
json_path_specification:
     Sconst         { $$ = $1; }
    ;

Would it make sense, fundamentally, to allow variables there? After
Andrew Gierth's analysis of this grammar problem, I understand that it's
not reasonable to expect JSON_TABLE() to support variable jsonpaths, but
maybe it would be feasible for everything else? From Andrew's changes to
the new grammar (see attached) it seems to me that at least that part is
possible. Or should I forget about trying to implement the other part?
By standard only string literals can be used in JSON path specifications.
But of course it is possible to allow to use variable jsonpath expressions in
SQL/JSON functions.

Attached patch implements this feature for JSON query functions, JSON_TABLE is
not supported now because it needs some refactoring.

I have pushed this commit to the separate branch because it is not finished yet:
https://github.com/postgrespro/sqljson/tree/sqljson_variable_json_path

--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
>From cba58ec1410eb99fc974d8a46013ce7331c93377 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <n.glu...@postgrespro.ru>
Date: Fri, 3 Nov 2017 14:15:51 +0300
Subject: [PATCH] Allow variable jsonpath specifications

---
 src/backend/executor/execExpr.c       |  7 +++++++
 src/backend/executor/execExprInterp.c |  5 ++---
 src/backend/nodes/copyfuncs.c         |  2 +-
 src/backend/parser/gram.y             | 11 ++++++-----
 src/backend/parser/parse_clause.c     | 35 ++++++++++++++++++++++++++++++-----
 src/backend/parser/parse_expr.c       | 19 +++++++++++++------
 src/backend/utils/adt/ruleutils.c     | 11 +++++++++--
 src/include/executor/execExpr.h       |  3 ++-
 src/include/nodes/parsenodes.h        |  2 +-
 src/include/nodes/primnodes.h         |  2 +-
 10 files changed, 72 insertions(+), 25 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 3cc8e12..86030b9 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -2042,6 +2042,13 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state,
 								&scratch.d.jsonexpr.raw_expr->value,
 								&scratch.d.jsonexpr.raw_expr->isnull);
 
+				scratch.d.jsonexpr.pathspec =
+					palloc(sizeof(*scratch.d.jsonexpr.pathspec));
+
+				ExecInitExprRec((Expr *) jexpr->path_spec, parent, state,
+								&scratch.d.jsonexpr.pathspec->value,
+								&scratch.d.jsonexpr.pathspec->isnull);
+
 				scratch.d.jsonexpr.formatted_expr =
 						ExecInitExpr((Expr *) jexpr->formatted_expr, parent);
 
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index b9d719d..36ef0fd 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -3897,7 +3897,7 @@ ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
 	*op->resnull = true;		/* until we get a result */
 	*op->resvalue = (Datum) 0;
 
-	if (op->d.jsonexpr.raw_expr->isnull)
+	if (op->d.jsonexpr.raw_expr->isnull || op->d.jsonexpr.pathspec->isnull)
 	{
 		/* execute domain checks for NULLs */
 		(void) ExecEvalJsonExprCoercion(op, econtext, res, op->resnull, isjsonb);
@@ -3905,8 +3905,7 @@ ExecEvalJson(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
 	}
 
 	item = op->d.jsonexpr.raw_expr->value;
-
-	path = DatumGetJsonPath(jexpr->path_spec->constvalue);
+	path = DatumGetJsonPath(op->d.jsonexpr.pathspec->value);
 
 	/* reset JSON path variable contexts */
 	foreach(lc, op->d.jsonexpr.args)
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 80c0e48..34fed1d 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2332,7 +2332,7 @@ _copyJsonCommon(const JsonCommon *from)
 	JsonCommon	   *newnode = makeNode(JsonCommon);
 
 	COPY_NODE_FIELD(expr);
-	COPY_STRING_FIELD(pathspec);
+	COPY_NODE_FIELD(pathspec);
 	COPY_STRING_FIELD(pathname);
 	COPY_NODE_FIELD(passing);
 	COPY_LOCATION_FIELD(location);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index adfe9b1..71a59d1 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -624,6 +624,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 					json_table_plan_cross
 					json_table_plan_primary
 					json_table_default_plan
+					json_path_specification
 
 %type <list>		json_arguments
 					json_passing_clause_opt
@@ -635,8 +636,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 
 %type <typnam>		json_returning_clause_opt
 
-%type <str>			json_path_specification
-					json_table_column_path_specification_clause_opt
+%type <str>			json_table_column_path_specification_clause_opt
 					json_table_path_name
 					json_as_path_name_clause_opt
 
@@ -845,6 +845,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
  */
 %nonassoc	UNBOUNDED		/* ideally should have same precedence as IDENT */
 %nonassoc	ERROR_P EMPTY_P DEFAULT ABSENT /* JSON error/empty behavior */
+%nonassoc	COLUMNS FALSE_P KEEP OMIT PASSING TRUE_P UNKNOWN
 %nonassoc	IDENT GENERATED NULL_P PARTITION RANGE ROWS PRECEDING FOLLOWING CUBE ROLLUP
 %left		Op OPERATOR		/* multi-character ops and user-defined operators */
 %left		'+' '-'
@@ -14472,7 +14473,7 @@ json_context_item:
 		;
 
 json_path_specification:
-			Sconst									{ $$ = $1; }
+			a_expr									{ $$ = $1; }
 		;
 
 json_as_path_name_clause_opt:
@@ -14770,7 +14771,7 @@ json_table_error_clause_opt:
 		;
 
 json_table_column_path_specification_clause_opt:
-			PATH json_path_specification			{ $$ = $2; }
+			PATH Sconst								{ $$ = $2; }
 			| /* EMPTY */ %prec json_table_column	{ $$ = NULL; }
 		;
 
@@ -14802,7 +14803,7 @@ json_table_formatted_column_definition:
 		;
 
 json_table_nested_columns:
-			NESTED path_opt json_path_specification
+			NESTED path_opt Sconst
 							json_as_path_name_clause_opt
 							json_table_columns_clause
 				{
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 227925e..fe9cb30 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -1088,6 +1088,18 @@ transformRangeTableSample(ParseState *pstate, RangeTableSample *rts)
 	return tablesample;
 }
 
+static Node *
+makeStringConst(char *str, int location)
+{
+	A_Const *n = makeNode(A_Const);
+
+	n->val.type = T_String;
+	n->val.val.str = str;
+	n->location = location;
+
+	return (Node *)n;
+}
+
 /*
  * Transform JSON_TABLE column
  *   - regular column into JSON_VALUE()
@@ -1101,6 +1113,7 @@ transformJsonTableColumn(JsonTableColumn *jtc, Node *contextItemExpr,
 	JsonValueExpr *jvexpr = makeNode(JsonValueExpr);
 	JsonCommon *common = makeNode(JsonCommon);
 	JsonOutput *output = makeNode(JsonOutput);
+	JsonPathSpec pathspec;
 
 	jfexpr->op = jtc->coltype == JTC_REGULAR ? IS_JSON_VALUE : IS_JSON_QUERY;
 	jfexpr->common = common;
@@ -1121,7 +1134,7 @@ transformJsonTableColumn(JsonTableColumn *jtc, Node *contextItemExpr,
 	common->passing = passingArgs;
 
 	if (jtc->pathspec)
-		common->pathspec = jtc->pathspec;
+		pathspec = jtc->pathspec;
 	else
 	{
 		/* Construct default path as '$."column_name"' */
@@ -1132,9 +1145,11 @@ transformJsonTableColumn(JsonTableColumn *jtc, Node *contextItemExpr,
 		appendStringInfoString(&path, "$.");
 		escape_json(&path, jtc->name);
 
-		common->pathspec = path.data;
+		pathspec = path.data;
 	}
 
+	common->pathspec = makeStringConst(pathspec, -1);
+
 	jvexpr->expr = (Expr *) contextItemExpr;
 	jvexpr->format.type = JS_FORMAT_DEFAULT;
 	jvexpr->format.encoding = JS_ENC_DEFAULT;
@@ -1628,6 +1643,7 @@ transformJsonTable(ParseState *pstate, JsonTable *jt)
 	JsonCommon *jscommon;
 	JsonTablePlan *plan = jt->plan;
 	char	   *rootPathName = jt->common->pathname;
+	char	   *rootPath;
 	bool		is_lateral;
 
 	cxt.table = jt;
@@ -1657,7 +1673,7 @@ transformJsonTable(ParseState *pstate, JsonTable *jt)
 	}
 
 	jscommon = copyObject(jt->common);
-	jscommon->pathspec = pstrdup("$");
+	jscommon->pathspec = makeStringConst(pstrdup("$"), -1);
 
 	jfe->op = IS_JSON_TABLE;
 	jfe->common = jscommon;
@@ -1681,10 +1697,19 @@ transformJsonTable(ParseState *pstate, JsonTable *jt)
 
 	cxt.contextItemTypid = exprType(tf->docexpr);
 
+	if (!IsA(jt->common->pathspec, A_Const) ||
+		castNode(A_Const, jt->common->pathspec)->val.type != T_String)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("only string constants supported in JSON_TABLE path specification"),
+				 parser_errposition(pstate,
+									exprLocation(jt->common->pathspec))));
+
+	rootPath = castNode(A_Const, jt->common->pathspec)->val.val.str;
+
 	tf->plan = (Node *) transformJsonTableColumns(pstate, &cxt, plan,
 												  jt->columns,
-												  jt->common->pathspec,
-												  &rootPathName,
+												  rootPath, &rootPathName,
 												  jt->common->location);
 
 	tf->ordinalitycol = -1;		/* undefine ordinality column number */
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 1ec3ae4..70da19a 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4353,7 +4353,7 @@ static JsonExpr *
 transformJsonExprCommon(ParseState *pstate, JsonFuncExpr *func)
 {
 	JsonExpr   *jsexpr = makeNode(JsonExpr);
-	Datum		jsonpath;
+	Node	   *pathspec;
 	JsonFormatType format;
 
 	if (func->common->pathname && func->op != IS_JSON_TABLE)
@@ -4383,12 +4383,19 @@ transformJsonExprCommon(ParseState *pstate, JsonFuncExpr *func)
 
 	jsexpr->format = func->common->expr->format;
 
-	/* parse JSON path string */
-	jsonpath = DirectFunctionCall1(jsonpath_in,
-									CStringGetDatum(func->common->pathspec));
+	pathspec = transformExprRecurse(pstate, func->common->pathspec);
 
-	jsexpr->path_spec = makeConst(JSONPATHOID, -1, InvalidOid, -1,
-								  jsonpath, false, false);
+	jsexpr->path_spec =
+		coerce_to_target_type(pstate, pathspec, exprType(pathspec),
+							  JSONPATHOID, -1,
+							  COERCION_EXPLICIT, COERCE_IMPLICIT_CAST,
+							  exprLocation(pathspec));
+	if (!jsexpr->path_spec)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATATYPE_MISMATCH),
+				 errmsg("JSON path expression must be type %s, not type %s",
+						"jsonpath", format_type_be(exprType(pathspec))),
+				 parser_errposition(pstate, exprLocation(pathspec))));
 
 	/* transform and coerce to json[b] passing arguments */
 	transformJsonPassingArgs(pstate, format, func->common->passing,
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 51c5a9d..48c7027 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -8834,7 +8834,10 @@ get_rule_expr(Node *node, deparse_context *context,
 
 				appendStringInfoString(buf, ", ");
 
-				get_const_expr(jexpr->path_spec, context, -1);
+				if (IsA(jexpr->path_spec, Const))
+					get_const_expr((Const *) jexpr->path_spec, context, -1);
+				else
+					get_rule_expr(jexpr->path_spec, context, false);
 
 				if (jexpr->passing.values)
 				{
@@ -9833,7 +9836,11 @@ get_json_table_columns(TableFunc *tf, JsonTableParentNode *node,
 								   " FORMAT JSONB" : " FORMAT JSON");
 
 		appendStringInfoString(buf, " PATH ");
-		get_const_expr(colexpr->path_spec, context, -1);
+
+		if (IsA(colexpr->path_spec, Const))
+			get_const_expr((Const *) colexpr->path_spec, context, -1);
+		else
+			get_rule_expr(colexpr->path_spec, context, false);
 
 		if (colexpr->wrapper == JSW_CONDITIONAL)
 			appendStringInfo(buf, " WITH CONDITIONAL WRAPPER");
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 96995d8..5bd4840 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -578,7 +578,8 @@ typedef struct ExprEvalStep
 			{
 				Datum		value;
 				bool		isnull;
-			}		   *raw_expr;			/* raw context item value */
+			}		   *raw_expr,			/* raw context item value */
+					   *pathspec;			/* path specification value */
 
 			ExprState  *formatted_expr;		/* formatted context item */
 			ExprState  *result_expr;		/* coerced to output type */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index afd2d9e..630715b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1475,7 +1475,7 @@ typedef struct JsonCommon
 {
 	NodeTag		type;
 	JsonValueExpr *expr;		/* context item expression */
-	JsonPathSpec pathspec;		/* JSON path specification */
+	Node	   *pathspec;		/* JSON path specification expression */
 	char	   *pathname;		/* path name, if any */
 	List	   *passing;		/* list of PASSING clause arguments, if any */
 	int			location;		/* token location, or -1 if unknown */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 15eee2b..1b7dc27 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1281,7 +1281,7 @@ typedef struct JsonExpr
 	bool		coerce_via_io;	/* coerce result using type input function */
 	Oid			coerce_via_io_collation; /* collation for conversion through I/O */
 	JsonFormat	format;			/* context item format (JSON/JSONB) */
-	Const	   *path_spec;		/* JSON path specification */
+	Node	   *path_spec;		/* JSON path specification expression */
 	JsonPassing	passing;		/* PASSING clause arguments */
 	JsonReturning returning;	/* RETURNING clause type/format info */
 	JsonBehavior on_empty;		/* ON EMPTY behavior */
-- 
2.7.4

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to