database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-17 17:02:08 +03:00
Code Activity

Code review for recent SQL/JSON commits

Browse Source 
- At the last minute and for no particularly good reason, I changed the
  WITHOUT token to be marked especially for lookahead, from the one in
  WITHOUT TIME to the one in WITHOUT UNIQUE.  Study of upcoming patches
  (where a new WITHOUT ARRAY WRAPPER clause is added) showed me that the
  former was better, so put it back the way the original patch had it.

- update exprTypmod() for JsonConstructorExpr to return the typmod of
  the RETURNING clause, as a comment there suggested.  Perhaps it's
  possible for this to make a difference with datetime types, but I
  didn't try to build a test case.

- The nodeFuncs.c support code for new nodes was calling walker()
  directly instead of the WALK() macro as introduced by commit 1c27d16e6e.
  Modernize that.  Also add exprLocation() support for a couple of nodes
  that missed it.  Lastly, reorder the code more sensibly.

The WITHOUT_LA -> WITHOUT change means that stored rules containing
either WITHOUT TIME ZONE or WITHOUT UNIQUE KEYS would change
representation.  Therefore, bump catversion.

Discussion: https://postgr.es/m/20230329181708.e64g2tpy7jyufqkr@alvherre.pgsql
This commit is contained in:
Alvaro Herrera
2023-04-04 14:04:30 +02:00
parent 8a2b1b1477
commit 71bfd1543f
5 changed files with 266 additions and 172 deletions
Download Patch File Download Diff File Expand all files Collapse all files

420
src/backend/nodes/nodeFuncs.c
View File

@ -218,6 +218,21 @@ exprType(const Node *expr)
else
type = XMLOID;
break;
case T_JsonValueExpr:
{
const JsonValueExpr *jve = (const JsonValueExpr *) expr;
type = exprType((Node *)
(jve->formatted_expr ? jve->formatted_expr :
jve->raw_expr));
}
break;
case T_JsonConstructorExpr:
type = ((const JsonConstructorExpr *) expr)->returning->typid;
break;
case T_JsonIsPredicate:
type = BOOLOID;
break;
case T_NullTest:
type = BOOLOID;
break;
@ -249,21 +264,6 @@ exprType(const Node *expr)
case T_PlaceHolderVar:
type = exprType((Node *) ((const PlaceHolderVar *) expr)->phexpr);
break;
case T_JsonValueExpr:
{
const JsonValueExpr *jve = (const JsonValueExpr *) expr;
type = exprType((Node *)
(jve->formatted_expr ? jve->formatted_expr :
jve->raw_expr));
}
break;
case T_JsonConstructorExpr:
type = ((const JsonConstructorExpr *) expr)->returning->typid;
break;
case T_JsonIsPredicate:
type = BOOLOID;
break;
default:
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(expr));
type = InvalidOid; /* keep compiler quiet */
@ -486,6 +486,10 @@ exprTypmod(const Node *expr)
return typmod;
}
break;
case T_JsonValueExpr:
return exprTypmod((Node *) ((const JsonValueExpr *) expr)->formatted_expr);
case T_JsonConstructorExpr:
return ((const JsonConstructorExpr *) expr)->returning->typmod;
case T_CoerceToDomain:
return ((const CoerceToDomain *) expr)->resulttypmod;
case T_CoerceToDomainValue:
@ -494,10 +498,6 @@ exprTypmod(const Node *expr)
return ((const SetToDefault *) expr)->typeMod;
case T_PlaceHolderVar:
return exprTypmod((Node *) ((const PlaceHolderVar *) expr)->phexpr);
case T_JsonValueExpr:
return exprTypmod((Node *) ((const JsonValueExpr *) expr)->formatted_expr);
case T_JsonConstructorExpr:
return -1; /* XXX maybe expr->returning->typmod? */
default:
break;
}
@ -942,6 +942,23 @@ exprCollation(const Node *expr)
else
coll = InvalidOid;
break;
case T_JsonValueExpr:
coll = exprCollation((Node *) ((const JsonValueExpr *) expr)->formatted_expr);
break;
case T_JsonConstructorExpr:
{
const JsonConstructorExpr *ctor = (const JsonConstructorExpr *) expr;
if (ctor->coercion)
coll = exprCollation((Node *) ctor->coercion);
else
coll = InvalidOid;
}
break;
case T_JsonIsPredicate:
/* IS JSON's result is boolean ... */
coll = InvalidOid; /* ... so it has no collation */
break;
case T_NullTest:
/* NullTest's result is boolean ... */
coll = InvalidOid; /* ... so it has no collation */
@ -973,22 +990,6 @@ exprCollation(const Node *expr)
case T_PlaceHolderVar:
coll = exprCollation((Node *) ((const PlaceHolderVar *) expr)->phexpr);
break;
case T_JsonValueExpr:
coll = exprCollation((Node *) ((const JsonValueExpr *) expr)->formatted_expr);
break;
case T_JsonConstructorExpr:
{
const JsonConstructorExpr *ctor = (const JsonConstructorExpr *) expr;
if (ctor->coercion)
coll = exprCollation((Node *) ctor->coercion);
else
coll = InvalidOid;
}
break;
case T_JsonIsPredicate:
coll = InvalidOid; /* result is always an boolean type */
break;
default:
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(expr));
coll = InvalidOid; /* keep compiler quiet */
@ -1171,6 +1172,24 @@ exprSetCollation(Node *expr, Oid collation)
(collation == DEFAULT_COLLATION_OID) :
(collation == InvalidOid));
break;
case T_JsonValueExpr:
exprSetCollation((Node *) ((JsonValueExpr *) expr)->formatted_expr,
collation);
break;
case T_JsonConstructorExpr:
{
JsonConstructorExpr *ctor = (JsonConstructorExpr *) expr;
if (ctor->coercion)
exprSetCollation((Node *) ctor->coercion, collation);
else
Assert(!OidIsValid(collation)); /* result is always a
* json[b] type */
}
break;
case T_JsonIsPredicate:
Assert(!OidIsValid(collation)); /* result is always boolean */
break;
case T_NullTest:
/* NullTest's result is boolean ... */
Assert(!OidIsValid(collation)); /* ... so never set a collation */
@ -1196,24 +1215,6 @@ exprSetCollation(Node *expr, Oid collation)
/* NextValueExpr's result is an integer type ... */
Assert(!OidIsValid(collation)); /* ... so never set a collation */
break;
case T_JsonValueExpr:
exprSetCollation((Node *) ((JsonValueExpr *) expr)->formatted_expr,
collation);
break;
case T_JsonConstructorExpr:
{
JsonConstructorExpr *ctor = (JsonConstructorExpr *) expr;
if (ctor->coercion)
exprSetCollation((Node *) ctor->coercion, collation);
else
Assert(!OidIsValid(collation)); /* result is always a
* json[b] type */
}
break;
case T_JsonIsPredicate:
Assert(!OidIsValid(collation)); /* result is always boolean */
break;
default:
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(expr));
break;
@ -1476,6 +1477,18 @@ exprLocation(const Node *expr)
exprLocation((Node *) xexpr->args));
}
break;
case T_JsonFormat:
loc = ((const JsonFormat *) expr)->location;
break;
case T_JsonValueExpr:
loc = exprLocation((Node *) ((const JsonValueExpr *) expr)->raw_expr);
break;
case T_JsonConstructorExpr:
loc = ((const JsonConstructorExpr *) expr)->location;
break;
case T_JsonIsPredicate:
loc = ((const JsonIsPredicate *) expr)->location;
break;
case T_NullTest:
{
const NullTest *nexpr = (const NullTest *) expr;
@ -1636,6 +1649,28 @@ exprLocation(const Node *expr)
case T_CommonTableExpr:
loc = ((const CommonTableExpr *) expr)->location;
break;
case T_JsonKeyValue:
/* just use the key's location */
loc = exprLocation((Node *) ((const JsonKeyValue *) expr)->key);
break;
case T_JsonObjectConstructor:
loc = ((const JsonObjectConstructor *) expr)->location;
break;
case T_JsonArrayConstructor:
loc = ((const JsonArrayConstructor *) expr)->location;
break;
case T_JsonArrayQueryConstructor:
loc = ((const JsonArrayQueryConstructor *) expr)->location;
break;
case T_JsonAggConstructor:
loc = ((const JsonAggConstructor *) expr)->location;
break;
case T_JsonObjectAgg:
loc = exprLocation((Node *) ((const JsonObjectAgg *) expr)->constructor);
break;
case T_JsonArrayAgg:
loc = exprLocation((Node *) ((const JsonArrayAgg *) expr)->constructor);
break;
case T_PlaceHolderVar:
/* just use argument's location */
loc = exprLocation((Node *) ((const PlaceHolderVar *) expr)->phexpr);
@ -1656,15 +1691,6 @@ exprLocation(const Node *expr)
case T_PartitionRangeDatum:
loc = ((const PartitionRangeDatum *) expr)->location;
break;
case T_JsonValueExpr:
loc = exprLocation((Node *) ((const JsonValueExpr *) expr)->raw_expr);
break;
case T_JsonConstructorExpr:
loc = ((const JsonConstructorExpr *) expr)->location;
break;
case T_JsonIsPredicate:
loc = ((const JsonIsPredicate *) expr)->location;
break;
default:
/* for any other node type it's just unknown... */
loc = -1;
@ -2190,6 +2216,30 @@ expression_tree_walker_impl(Node *node,
return true;
}
break;
case T_JsonValueExpr:
{
JsonValueExpr *jve = (JsonValueExpr *) node;
if (WALK(jve->raw_expr))
return true;
if (WALK(jve->formatted_expr))
return true;
}
break;
case T_JsonConstructorExpr:
{
JsonConstructorExpr *ctor = (JsonConstructorExpr *) node;
if (WALK(ctor->args))
return true;
if (WALK(ctor->func))
return true;
if (WALK(ctor->coercion))
return true;
}
break;
case T_JsonIsPredicate:
return WALK(((JsonIsPredicate *) node)->expr);
case T_NullTest:
return WALK(((NullTest *) node)->arg);
case T_BooleanTest:
@ -2242,6 +2292,73 @@ expression_tree_walker_impl(Node *node,
return true;
}
break;
case T_JsonKeyValue:
{
JsonKeyValue *kv = (JsonKeyValue *) node;
if (WALK(kv->key))
return true;
if (WALK(kv->value))
return true;
}
break;
case T_JsonObjectConstructor:
{
JsonObjectConstructor *ctor = (JsonObjectConstructor *) node;
if (LIST_WALK(ctor->exprs))
return true;
}
break;
case T_JsonArrayConstructor:
{
JsonArrayConstructor *ctor = (JsonArrayConstructor *) node;
if (LIST_WALK(ctor->exprs))
return true;
}
break;
case T_JsonArrayQueryConstructor:
{
JsonArrayQueryConstructor *ctor = (JsonArrayQueryConstructor *) node;
if (WALK(ctor->query))
return true;
}
break;
case T_JsonAggConstructor:
{
JsonAggConstructor *ctor = (JsonAggConstructor *) node;
if (WALK(ctor->agg_filter))
return true;
if (WALK(ctor->agg_order))
return true;
if (WALK(ctor->over))
return true;
}
break;
case T_JsonObjectAgg:
{
JsonObjectAgg *ctor = (JsonObjectAgg *) node;
if (WALK(ctor->constructor))
return true;
if (WALK(ctor->arg))
return true;
}
break;
case T_JsonArrayAgg:
{
JsonArrayAgg *ctor = (JsonArrayAgg *) node;
if (WALK(ctor->constructor))
return true;
if (WALK(ctor->arg))
return true;
}
break;
case T_PartitionBoundSpec:
{
PartitionBoundSpec *pbs = (PartitionBoundSpec *) node;
@ -2396,30 +2513,6 @@ expression_tree_walker_impl(Node *node,
return true;
}
break;
case T_JsonValueExpr:
{
JsonValueExpr *jve = (JsonValueExpr *) node;
if (WALK(jve->raw_expr))
return true;
if (WALK(jve->formatted_expr))
return true;
}
break;
case T_JsonConstructorExpr:
{
JsonConstructorExpr *ctor = (JsonConstructorExpr *) node;
if (WALK(ctor->args))
return true;
if (WALK(ctor->func))
return true;
if (WALK(ctor->coercion))
return true;
}
break;
case T_JsonIsPredicate:
return walker(((JsonIsPredicate *) node)->expr, context);
default:
elog(ERROR, "unrecognized node type: %d",
(int) nodeTag(node));
@ -2743,6 +2836,7 @@ expression_tree_mutator_impl(Node *node,
break;
case T_Param:
case T_CaseTestExpr:
case T_JsonFormat:
case T_CoerceToDomainValue:
case T_SetToDefault:
case T_CurrentOfExpr:
@ -2750,7 +2844,6 @@ expression_tree_mutator_impl(Node *node,
case T_RangeTblRef:
case T_SortGroupClause:
case T_CTESearchClause:
case T_JsonFormat:
return (Node *) copyObject(node);
case T_WithCheckOption:
{
@ -3099,6 +3192,52 @@ expression_tree_mutator_impl(Node *node,
return (Node *) newnode;
}
break;
case T_JsonReturning:
{
JsonReturning *jr = (JsonReturning *) node;
JsonReturning *newnode;
FLATCOPY(newnode, jr, JsonReturning);
MUTATE(newnode->format, jr->format, JsonFormat *);
return (Node *) newnode;
}
case T_JsonValueExpr:
{
JsonValueExpr *jve = (JsonValueExpr *) node;
JsonValueExpr *newnode;
FLATCOPY(newnode, jve, JsonValueExpr);
MUTATE(newnode->raw_expr, jve->raw_expr, Expr *);
MUTATE(newnode->formatted_expr, jve->formatted_expr, Expr *);
MUTATE(newnode->format, jve->format, JsonFormat *);
return (Node *) newnode;
}
case T_JsonConstructorExpr:
{
JsonConstructorExpr *jce = (JsonConstructorExpr *) node;
JsonConstructorExpr *newnode;
FLATCOPY(newnode, jce, JsonConstructorExpr);
MUTATE(newnode->args, jce->args, List *);
MUTATE(newnode->func, jce->func, Expr *);
MUTATE(newnode->coercion, jce->coercion, Expr *);
MUTATE(newnode->returning, jce->returning, JsonReturning *);
return (Node *) newnode;
}
case T_JsonIsPredicate:
{
JsonIsPredicate *pred = (JsonIsPredicate *) node;
JsonIsPredicate *newnode;
FLATCOPY(newnode, pred, JsonIsPredicate);
MUTATE(newnode->expr, pred->expr, Node *);
MUTATE(newnode->format, pred->format, JsonFormat *);
return (Node *) newnode;
}
case T_NullTest:
{
NullTest *ntest = (NullTest *) node;
@ -3394,51 +3533,6 @@ expression_tree_mutator_impl(Node *node,
return (Node *) newnode;
}
break;
case T_JsonReturning:
{
JsonReturning *jr = (JsonReturning *) node;
JsonReturning *newnode;
FLATCOPY(newnode, jr, JsonReturning);
MUTATE(newnode->format, jr->format, JsonFormat *);
return (Node *) newnode;
}
case T_JsonValueExpr:
{
JsonValueExpr *jve = (JsonValueExpr *) node;
JsonValueExpr *newnode;
FLATCOPY(newnode, jve, JsonValueExpr);
MUTATE(newnode->raw_expr, jve->raw_expr, Expr *);
MUTATE(newnode->formatted_expr, jve->formatted_expr, Expr *);
MUTATE(newnode->format, jve->format, JsonFormat *);
return (Node *) newnode;
}
case T_JsonConstructorExpr:
{
JsonConstructorExpr *jve = (JsonConstructorExpr *) node;
JsonConstructorExpr *newnode;
FLATCOPY(newnode, jve, JsonConstructorExpr);
MUTATE(newnode->args, jve->args, List *);
MUTATE(newnode->func, jve->func, Expr *);
MUTATE(newnode->coercion, jve->coercion, Expr *);
MUTATE(newnode->returning, jve->returning, JsonReturning *);
return (Node *) newnode;
}
case T_JsonIsPredicate:
{
JsonIsPredicate *pred = (JsonIsPredicate *) node;
JsonIsPredicate *newnode;
FLATCOPY(newnode, pred, JsonIsPredicate);
MUTATE(newnode->expr, pred->expr, Node *);
return (Node *) newnode;
}
default:
elog(ERROR, "unrecognized node type: %d",
(int) nodeTag(node));
@ -3700,6 +3794,7 @@ raw_expression_tree_walker_impl(Node *node,
switch (nodeTag(node))
{
case T_JsonFormat:
case T_SetToDefault:
case T_CurrentOfExpr:
case T_Integer:
@ -3710,7 +3805,6 @@ raw_expression_tree_walker_impl(Node *node,
case T_ParamRef:
case T_A_Const:
case T_A_Star:
case T_JsonFormat:
/* primitive node types with no subnodes */
break;
case T_Alias:
@ -3769,6 +3863,36 @@ raw_expression_tree_walker_impl(Node *node,
return true;
}
break;
case T_JsonReturning:
return WALK(((JsonReturning *) node)->format);
case T_JsonValueExpr:
{
JsonValueExpr *jve = (JsonValueExpr *) node;
if (WALK(jve->raw_expr))
return true;
if (WALK(jve->formatted_expr))
return true;
if (WALK(jve->format))
return true;
}
break;
case T_JsonConstructorExpr:
{
JsonConstructorExpr *ctor = (JsonConstructorExpr *) node;
if (WALK(ctor->args))
return true;
if (WALK(ctor->func))
return true;
if (WALK(ctor->coercion))
return true;
if (WALK(ctor->returning))
return true;
}
break;
case T_JsonIsPredicate:
return WALK(((JsonIsPredicate *) node)->expr);
case T_NullTest:
return WALK(((NullTest *) node)->arg);
case T_BooleanTest:
@ -4173,34 +4297,6 @@ raw_expression_tree_walker_impl(Node *node,
case T_CommonTableExpr:
/* search_clause and cycle_clause are not interesting here */
return WALK(((CommonTableExpr *) node)->ctequery);
case T_JsonReturning:
return WALK(((JsonReturning *) node)->format);
case T_JsonValueExpr:
{
JsonValueExpr *jve = (JsonValueExpr *) node;
if (WALK(jve->raw_expr))
return true;
if (WALK(jve->formatted_expr))
return true;
if (WALK(jve->format))
return true;
}
break;
case T_JsonConstructorExpr:
{
JsonConstructorExpr *ctor = (JsonConstructorExpr *) node;
if (WALK(ctor->args))
return true;
if (WALK(ctor->func))
return true;
if (WALK(ctor->coercion))
return true;
if (WALK(ctor->returning))
return true;
}
break;
case T_JsonOutput:
{
JsonOutput *out = (JsonOutput *) node;
@ -4285,8 +4381,6 @@ raw_expression_tree_walker_impl(Node *node,
return true;
}
break;
case T_JsonIsPredicate:
return walker(((JsonIsPredicate *) node)->expr, context);
default:
elog(ERROR, "unrecognized node type: %d",
(int) nodeTag(node));

8
src/backend/parser/gram.y
View File

@ -824,7 +824,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
/* SQL/JSON related keywords */
%nonassoc UNIQUE JSON
%nonassoc KEYS OBJECT_P SCALAR VALUE_P
%nonassoc WITH WITHOUT_LA
%nonassoc WITH WITHOUT
/*
* To support target_el without AS, it used to be necessary to assign IDENT an
@ -14313,7 +14313,7 @@ ConstInterval:
opt_timezone:
WITH_LA TIME ZONE { $$ = true; }
| WITHOUT TIME ZONE { $$ = false; }
| WITHOUT_LA TIME ZONE { $$ = false; }
| /*EMPTY*/ { $$ = false; }
;
@ -16464,8 +16464,8 @@ json_predicate_type_constraint:
json_key_uniqueness_constraint_opt:
WITH UNIQUE KEYS { $$ = true; }
| WITH UNIQUE { $$ = true; }
| WITHOUT_LA UNIQUE KEYS { $$ = false; }
| WITHOUT_LA UNIQUE { $$ = false; }
| WITHOUT UNIQUE KEYS { $$ = false; }
| WITHOUT UNIQUE { $$ = false; }
| /* EMPTY */ %prec KEYS { $$ = false; }
;

4
src/backend/parser/parser.c
View File

@ -241,10 +241,10 @@ base_yylex(YYSTYPE *lvalp, YYLTYPE *llocp, core_yyscan_t yyscanner)
break;
case WITHOUT:
/* Replace WITHOUT by WITHOUT_LA if it's followed by UNIQUE */
/* Replace WITHOUT by WITHOUT_LA if it's followed by TIME */
switch (next_token)
{
case UNIQUE:
case TIME:
cur_token = WITHOUT_LA;
break;
}

2
src/include/catalog/catversion.h
View File

@ -57,6 +57,6 @@
*/
/* yyyymmddN */
#define CATALOG_VERSION_NO 202303311
#define CATALOG_VERSION_NO 202304041
#endif

4
src/interfaces/ecpg/preproc/parser.c
View File

@ -159,10 +159,10 @@ filtered_base_yylex(void)
break;
case WITHOUT:
/* Replace WITHOUT by WITHOUT_LA if it's followed by UNIQUE */
/* Replace WITHOUT by WITHOUT_LA if it's followed by TIME */
switch (next_token)
{
case UNIQUE:
case TIME:
cur_token = WITHOUT_LA;
break;
}