Fix inconsistencies and style issues in new SQL/JSON code

alvherre · alvherre · commit 60966f56c3e4 · 2023-03-30T21:06:31.000+02:00
Reported by Alexander Lakhin. Discussion: https://postgr.es/m/60483139-5c34-851d-baee-6c0d014e1710@gmail.com
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
@@ -4479,10 +4479,7 @@ ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op,
 										  jcstate->constructor->absent_on_null,
 										  jcstate->constructor->unique);
 	else
-	{
-		res = (Datum) 0;
 		elog(ERROR, "invalid JsonConstructorExpr type %d", ctor->type);
-	}
 
 	*op->resvalue = res;
 	*op->resnull = isnull;
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
@@ -3138,7 +3138,7 @@ makeJsonByteaToTextConversion(Node *expr, JsonFormat *format, int location)
 }
 
 /*
- * Make CaseTestExpr node.
+ * Make a CaseTestExpr node.
  */
 static Node *
 makeCaseTestExpr(Node *expr)
@@ -3456,6 +3456,9 @@ coerceJsonFuncExpr(ParseState *pstate, Node *expr,
 	return res;
 }
 
+/*
+ * Make a JsonConstructorExpr node.
+ */
 static Node *
 makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type,
 						List *args, Expr *fexpr, JsonReturning *returning,
@@ -3464,8 +3467,6 @@ makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type,
 	JsonConstructorExpr *jsctor = makeNode(JsonConstructorExpr);
 	Node	   *placeholder;
 	Node	   *coercion;
-	Oid			intermediate_typid =
-	returning->format->format_type == JS_FORMAT_JSONB ? JSONBOID : JSONOID;
 
 	jsctor->args = args;
 	jsctor->func = fexpr;
@@ -3481,7 +3482,8 @@ makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type,
 	{
 		CaseTestExpr *cte = makeNode(CaseTestExpr);
 
-		cte->typeId = intermediate_typid;
+		cte->typeId = returning->format->format_type == JS_FORMAT_JSONB ?
+			JSONBOID : JSONOID;
 		cte->typeMod = -1;
 		cte->collation = InvalidOid;
 
@@ -3501,7 +3503,7 @@ makeJsonConstructorExpr(ParseState *pstate, JsonConstructorType type,
  *
  * JSON_OBJECT() is transformed into json[b]_build_object[_ext]() call
  * depending on the output JSON format. The first two arguments of
- * json[b]_build_object_ext() are absent_on_null and check_key_uniqueness.
+ * json[b]_build_object_ext() are absent_on_null and check_unique.
  *
  * Then function call result is coerced to the target type.
  */
@@ -3615,9 +3617,11 @@ transformJsonAggConstructor(ParseState *pstate, JsonAggConstructor *agg_ctor,
 {
 	Oid			aggfnoid;
 	Node	   *node;
-	Expr	   *aggfilter = agg_ctor->agg_filter ? (Expr *)
-	transformWhereClause(pstate, agg_ctor->agg_filter,
-						 EXPR_KIND_FILTER, "FILTER") : NULL;
+	Expr	   *aggfilter;
+
+	aggfilter = agg_ctor->agg_filter ? (Expr *)
+		transformWhereClause(pstate, agg_ctor->agg_filter,
+							 EXPR_KIND_FILTER, "FILTER") : NULL;
 
 	aggfnoid = DatumGetInt32(DirectFunctionCall1(regprocin,
 												 CStringGetDatum(aggfn)));
@@ -3631,10 +3635,10 @@ transformJsonAggConstructor(ParseState *pstate, JsonAggConstructor *agg_ctor,
 		wfunc->wintype = aggtype;
 		/* wincollid and inputcollid will be set by parse_collate.c */
 		wfunc->args = args;
+		wfunc->aggfilter = aggfilter;
 		/* winref will be set by transformWindowFuncCall */
 		wfunc->winstar = false;
 		wfunc->winagg = true;
-		wfunc->aggfilter = aggfilter;
 		wfunc->location = agg_ctor->location;
 
 		/*
@@ -3659,16 +3663,19 @@ transformJsonAggConstructor(ParseState *pstate, JsonAggConstructor *agg_ctor,
 		aggref->aggtype = aggtype;
 
 		/* aggcollid and inputcollid will be set by parse_collate.c */
-		aggref->aggtranstype = InvalidOid;	/* will be set by planner */
+		/* aggtranstype will be set by planner */
 		/* aggargtypes will be set by transformAggregateCall */
 		/* aggdirectargs and args will be set by transformAggregateCall */
 		/* aggorder and aggdistinct will be set by transformAggregateCall */
 		aggref->aggfilter = aggfilter;
 		aggref->aggstar = false;
 		aggref->aggvariadic = false;
 		aggref->aggkind = AGGKIND_NORMAL;
+		aggref->aggpresorted = false;
 		/* agglevelsup will be set by transformAggregateCall */
 		aggref->aggsplit = AGGSPLIT_SIMPLE; /* planner might change this */
+		aggref->aggno = -1;		/* planner will set aggno and aggtransno */
+		aggref->aggtransno = -1;
 		aggref->location = agg_ctor->location;
 
 		transformAggregateCall(pstate, aggref, args, agg_ctor->agg_order, false);
@@ -3685,7 +3692,7 @@ transformJsonAggConstructor(ParseState *pstate, JsonAggConstructor *agg_ctor,
  * Transform JSON_OBJECTAGG() aggregate function.
  *
  * JSON_OBJECTAGG() is transformed into
- * json[b]_objectagg(key, value, absent_on_null, check_unique) call depending on
+ * json[b]_objectagg[_unique][_strict](key, value) call depending on
  * the output JSON format.  Then the function call result is coerced to the
  * target output type.
  */
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
@@ -10654,7 +10654,7 @@ get_json_constructor(JsonConstructorExpr *ctor, deparse_context *context,
 			funcname = "JSON_ARRAY";
 			break;
 		default:
-			elog(ERROR, "invalid JsonConstructorExprType %d", ctor->type);
+			elog(ERROR, "invalid JsonConstructorType %d", ctor->type);
 	}
 
 	appendStringInfo(buf, "%s(", funcname);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
@@ -1809,7 +1809,7 @@ typedef struct JsonObjectAgg
 
 /*
  * JsonArrayAgg -
- *		untransformed representation of JSON_ARRRAYAGG()
+ *		untransformed representation of JSON_ARRAYAGG()
  */
 typedef struct JsonArrayAgg
 {
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
@@ -1548,7 +1548,7 @@ typedef struct JsonReturning
 
 /*
  * JsonValueExpr -
- *		representation of JSON value expression (expr [FORMAT json_format])
+ *		representation of JSON value expression (expr [FORMAT JsonFormat])
  */
 typedef struct JsonValueExpr
 {

Original file line number	Diff line number	Diff line change
`@@ -10654,7 +10654,7 @@ get_json_constructor(JsonConstructorExpr ctor, deparse_context context,`
`10654`	`10654`	`funcname = "JSON_ARRAY";`
`10655`	`10655`	`break;`
`10656`	`10656`	`default:`
`10657`		`- elog(ERROR, "invalid JsonConstructorExprType %d", ctor->type);`
	`10657`	`+ elog(ERROR, "invalid JsonConstructorType %d", ctor->type);`
`10658`	`10658`	`}`
`10659`	`10659`
`10660`	`10660`	`appendStringInfo(buf, "%s(", funcname);`
Original file line number	Diff line number	Diff line change
`@@ -1809,7 +1809,7 @@ typedef struct JsonObjectAgg`
`1809`	`1809`
`1810`	`1810`	`/*`
`1811`	`1811`	`* JsonArrayAgg -`
`1812`		`- * untransformed representation of JSON_ARRRAYAGG()`
	`1812`	`+ * untransformed representation of JSON_ARRAYAGG()`
`1813`	`1813`	`*/`
`1814`	`1814`	`typedef struct JsonArrayAgg`
`1815`	`1815`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1548,7 +1548,7 @@ typedef struct JsonReturning`
`1548`	`1548`
`1549`	`1549`	`/*`
`1550`	`1550`	`* JsonValueExpr -`
`1551`		`- * representation of JSON value expression (expr [FORMAT json_format])`
	`1551`	`+ * representation of JSON value expression (expr [FORMAT JsonFormat])`
`1552`	`1552`	`*/`
`1553`	`1553`	`typedef struct JsonValueExpr`
`1554`	`1554`	`{`