Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitdecb08e

Browse files
committed
Code review for NextValueExpr expression node type.
Add missing infrastructure for this node type, notably in ruleutils.c whereits lack could demonstrably cause EXPLAIN to fail. Add outfuncs/readfuncssupport. (outfuncs support is useful today for debugging purposes. Thereadfuncs support may never be needed, since at present it would onlymatter for parallel query and NextValueExpr should never appear in aparallelizable query; but it seems like a bad idea to have a primnode typethat isn't fully supported here.) Teach planner infrastructure thatNextValueExpr is a volatile, parallel-unsafe, non-leaky expression nodewith cost cpu_operator_cost. Given its limited scope of usage, there*might* be no live bug today from the lack of that knowledge, but it'scertainly going to bite us on the rear someday. Teach pg_stat_statementsabout the new node type, too.While at it, also teach cost_qual_eval() that MinMaxExpr, SQLValueFunction,XmlExpr, and CoerceToDomain should be charged as cpu_operator_cost.Failing to do this for SQLValueFunction was an oversight in my commit0bb51aa. The others are longer-standing oversights, but no time like thepresent to fix them. (In principle, CoerceToDomain could have cost muchhigher than this, but it doesn't presently seem worth trying to examine thedomain's constraints here.)Modify execExprInterp.c to execute NextValueExpr as an out-of-linefunction; it seems quite unlikely to me that it's worth insisting thatit be inlined in all expression eval methods. Besides, providing theout-of-line function doesn't stop anyone from inlining if they want to.Adjust some places where NextValueExpr support had been inserted with theaid of a dartboard rather than keeping it in the same order as elsewhere.Discussion:https://postgr.es/m/23862.1499981661@sss.pgh.pa.us
1 parentc95275f commitdecb08e

File tree

12 files changed

+147
-47
lines changed

12 files changed

+147
-47
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2763,6 +2763,14 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)
27632763
APP_JUMB(ce->cursor_param);
27642764
}
27652765
break;
2766+
caseT_NextValueExpr:
2767+
{
2768+
NextValueExpr*nve= (NextValueExpr*)node;
2769+
2770+
APP_JUMB(nve->seqid);
2771+
APP_JUMB(nve->typeId);
2772+
}
2773+
break;
27662774
caseT_InferenceElem:
27672775
{
27682776
InferenceElem*ie= (InferenceElem*)node;

‎src/backend/catalog/dependency.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,6 +1791,13 @@ find_expr_references_walker(Node *node,
17911791
add_object_address(OCLASS_TYPE,cd->resulttype,0,
17921792
context->addrs);
17931793
}
1794+
elseif (IsA(node,NextValueExpr))
1795+
{
1796+
NextValueExpr*nve= (NextValueExpr*)node;
1797+
1798+
add_object_address(OCLASS_CLASS,nve->seqid,0,
1799+
context->addrs);
1800+
}
17941801
elseif (IsA(node,OnConflictExpr))
17951802
{
17961803
OnConflictExpr*onconflict= (OnConflictExpr*)node;
@@ -1942,13 +1949,6 @@ find_expr_references_walker(Node *node,
19421949
context->addrs);
19431950
/* fall through to examine arguments */
19441951
}
1945-
elseif (IsA(node,NextValueExpr))
1946-
{
1947-
NextValueExpr*nve= (NextValueExpr*)node;
1948-
1949-
add_object_address(OCLASS_CLASS,nve->seqid,0,
1950-
context->addrs);
1951-
}
19521952

19531953
returnexpression_tree_walker(node,find_expr_references_walker,
19541954
(void*)context);

‎src/backend/executor/execExprInterp.c

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,21 +1232,11 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
12321232

12331233
EEO_CASE(EEOP_NEXTVALUEEXPR)
12341234
{
1235-
switch (op->d.nextvalueexpr.seqtypid)
1236-
{
1237-
caseINT2OID:
1238-
*op->resvalue=Int16GetDatum((int16)nextval_internal(op->d.nextvalueexpr.seqid, false));
1239-
break;
1240-
caseINT4OID:
1241-
*op->resvalue=Int32GetDatum((int32)nextval_internal(op->d.nextvalueexpr.seqid, false));
1242-
break;
1243-
caseINT8OID:
1244-
*op->resvalue=Int64GetDatum((int64)nextval_internal(op->d.nextvalueexpr.seqid, false));
1245-
break;
1246-
default:
1247-
elog(ERROR,"unsupported sequence type %u",op->d.nextvalueexpr.seqtypid);
1248-
}
1249-
*op->resnull= false;
1235+
/*
1236+
* Doesn't seem worthwhile to have an inline implementation
1237+
* efficiency-wise.
1238+
*/
1239+
ExecEvalNextValueExpr(state,op);
12501240

12511241
EEO_NEXT();
12521242
}
@@ -1989,6 +1979,32 @@ ExecEvalCurrentOfExpr(ExprState *state, ExprEvalStep *op)
19891979
errmsg("WHERE CURRENT OF is not supported for this table type")));
19901980
}
19911981

1982+
/*
1983+
* Evaluate NextValueExpr.
1984+
*/
1985+
void
1986+
ExecEvalNextValueExpr(ExprState*state,ExprEvalStep*op)
1987+
{
1988+
int64newval=nextval_internal(op->d.nextvalueexpr.seqid, false);
1989+
1990+
switch (op->d.nextvalueexpr.seqtypid)
1991+
{
1992+
caseINT2OID:
1993+
*op->resvalue=Int16GetDatum((int16)newval);
1994+
break;
1995+
caseINT4OID:
1996+
*op->resvalue=Int32GetDatum((int32)newval);
1997+
break;
1998+
caseINT8OID:
1999+
*op->resvalue=Int64GetDatum((int64)newval);
2000+
break;
2001+
default:
2002+
elog(ERROR,"unsupported sequence type %u",
2003+
op->d.nextvalueexpr.seqtypid);
2004+
}
2005+
*op->resnull= false;
2006+
}
2007+
19922008
/*
19932009
* Evaluate NullTest / IS NULL for rows.
19942010
*/

‎src/backend/nodes/nodeFuncs.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,10 +1642,10 @@ set_sa_opfuncid(ScalarArrayOpExpr *opexpr)
16421642
* for themselves, in case additional checks should be made, or because they
16431643
* have special rules about which parts of the tree need to be visited.
16441644
*
1645-
* Note: we ignore MinMaxExpr, SQLValueFunction, XmlExpr,andCoerceToDomain
1646-
* nodes, because they do not contain SQL function OIDs. However, they can
1647-
* invoke SQL-visible functions, so callers should take thought about how to
1648-
* treat them.
1645+
* Note: we ignore MinMaxExpr, SQLValueFunction, XmlExpr, CoerceToDomain,
1646+
*and NextValueExprnodes, because they do not contain SQL function OIDs.
1647+
*However, they caninvoke SQL-visible functions, so callers should take
1648+
*thought about how totreat them.
16491649
*/
16501650
bool
16511651
check_functions_in_node(Node*node,check_function_callbackchecker,
@@ -1865,12 +1865,12 @@ expression_tree_walker(Node *node,
18651865
caseT_Var:
18661866
caseT_Const:
18671867
caseT_Param:
1868-
caseT_CoerceToDomainValue:
18691868
caseT_CaseTestExpr:
1869+
caseT_SQLValueFunction:
1870+
caseT_CoerceToDomainValue:
18701871
caseT_SetToDefault:
18711872
caseT_CurrentOfExpr:
18721873
caseT_NextValueExpr:
1873-
caseT_SQLValueFunction:
18741874
caseT_RangeTblRef:
18751875
caseT_SortGroupClause:
18761876
/* primitive node types with no expression subnodes */
@@ -2461,12 +2461,12 @@ expression_tree_mutator(Node *node,
24612461
}
24622462
break;
24632463
caseT_Param:
2464-
caseT_CoerceToDomainValue:
24652464
caseT_CaseTestExpr:
2465+
caseT_SQLValueFunction:
2466+
caseT_CoerceToDomainValue:
24662467
caseT_SetToDefault:
24672468
caseT_CurrentOfExpr:
24682469
caseT_NextValueExpr:
2469-
caseT_SQLValueFunction:
24702470
caseT_RangeTblRef:
24712471
caseT_SortGroupClause:
24722472
return (Node*)copyObject(node);

‎src/backend/nodes/outfuncs.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,6 +1610,15 @@ _outCurrentOfExpr(StringInfo str, const CurrentOfExpr *node)
16101610
WRITE_INT_FIELD(cursor_param);
16111611
}
16121612

1613+
staticvoid
1614+
_outNextValueExpr(StringInfostr,constNextValueExpr*node)
1615+
{
1616+
WRITE_NODE_TYPE("NEXTVALUEEXPR");
1617+
1618+
WRITE_OID_FIELD(seqid);
1619+
WRITE_OID_FIELD(typeId);
1620+
}
1621+
16131622
staticvoid
16141623
_outInferenceElem(StringInfostr,constInferenceElem*node)
16151624
{
@@ -3872,6 +3881,9 @@ outNode(StringInfo str, const void *obj)
38723881
caseT_CurrentOfExpr:
38733882
_outCurrentOfExpr(str,obj);
38743883
break;
3884+
caseT_NextValueExpr:
3885+
_outNextValueExpr(str,obj);
3886+
break;
38753887
caseT_InferenceElem:
38763888
_outInferenceElem(str,obj);
38773889
break;

‎src/backend/nodes/readfuncs.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,6 +1202,20 @@ _readCurrentOfExpr(void)
12021202
READ_DONE();
12031203
}
12041204

1205+
/*
1206+
* _readNextValueExpr
1207+
*/
1208+
staticNextValueExpr*
1209+
_readNextValueExpr(void)
1210+
{
1211+
READ_LOCALS(NextValueExpr);
1212+
1213+
READ_OID_FIELD(seqid);
1214+
READ_OID_FIELD(typeId);
1215+
1216+
READ_DONE();
1217+
}
1218+
12051219
/*
12061220
* _readInferenceElem
12071221
*/
@@ -2517,6 +2531,8 @@ parseNodeString(void)
25172531
return_value=_readSetToDefault();
25182532
elseif (MATCH("CURRENTOFEXPR",13))
25192533
return_value=_readCurrentOfExpr();
2534+
elseif (MATCH("NEXTVALUEEXPR",13))
2535+
return_value=_readNextValueExpr();
25202536
elseif (MATCH("INFERENCEELEM",13))
25212537
return_value=_readInferenceElem();
25222538
elseif (MATCH("TARGETENTRY",11))

‎src/backend/optimizer/path/costsize.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3627,6 +3627,15 @@ cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
36273627
cpu_operator_cost;
36283628
}
36293629
}
3630+
elseif (IsA(node,MinMaxExpr)||
3631+
IsA(node,SQLValueFunction)||
3632+
IsA(node,XmlExpr)||
3633+
IsA(node,CoerceToDomain)||
3634+
IsA(node,NextValueExpr))
3635+
{
3636+
/* Treat all these as having cost 1 */
3637+
context->total.per_tuple+=cpu_operator_cost;
3638+
}
36303639
elseif (IsA(node,CurrentOfExpr))
36313640
{
36323641
/* Report high cost to prevent selection of anything but TID scan */

‎src/backend/optimizer/util/clauses.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,12 @@ contain_mutable_functions_walker(Node *node, void *context)
902902
return true;
903903
}
904904

905+
if (IsA(node,NextValueExpr))
906+
{
907+
/* NextValueExpr is volatile */
908+
return true;
909+
}
910+
905911
/*
906912
* It should be safe to treat MinMaxExpr as immutable, because it will
907913
* depend on a non-cross-type btree comparison function, and those should
@@ -969,6 +975,12 @@ contain_volatile_functions_walker(Node *node, void *context)
969975
context))
970976
return true;
971977

978+
if (IsA(node,NextValueExpr))
979+
{
980+
/* NextValueExpr is volatile */
981+
return true;
982+
}
983+
972984
/*
973985
* See notes in contain_mutable_functions_walker about why we treat
974986
* MinMaxExpr, XmlExpr, and CoerceToDomain as immutable, while
@@ -1019,6 +1031,8 @@ contain_volatile_functions_not_nextval_walker(Node *node, void *context)
10191031
* See notes in contain_mutable_functions_walker about why we treat
10201032
* MinMaxExpr, XmlExpr, and CoerceToDomain as immutable, while
10211033
* SQLValueFunction is stable. Hence, none of them are of interest here.
1034+
* Also, since we're intentionally ignoring nextval(), presumably we
1035+
* should ignore NextValueExpr.
10221036
*/
10231037

10241038
/* Recurse to check arguments */
@@ -1146,14 +1160,20 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context)
11461160
* contain a parallel-unsafe function; but useful constraints probably
11471161
* never would have such, and assuming they do would cripple use of
11481162
* parallel query in the presence of domain types.) SQLValueFunction
1149-
* should be safe in all cases.
1163+
* should be safe in all cases. NextValueExpr is parallel-unsafe.
11501164
*/
11511165
if (IsA(node,CoerceToDomain))
11521166
{
11531167
if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED,context))
11541168
return true;
11551169
}
11561170

1171+
if (IsA(node,NextValueExpr))
1172+
{
1173+
if (max_parallel_hazard_test(PROPARALLEL_UNSAFE,context))
1174+
return true;
1175+
}
1176+
11571177
/*
11581178
* As a notational convenience for callers, look through RestrictInfo.
11591179
*/
@@ -1495,6 +1515,7 @@ contain_leaked_vars_walker(Node *node, void *context)
14951515
caseT_SQLValueFunction:
14961516
caseT_NullTest:
14971517
caseT_BooleanTest:
1518+
caseT_NextValueExpr:
14981519
caseT_List:
14991520

15001521
/*

‎src/backend/utils/adt/ruleutils.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7283,6 +7283,7 @@ isSimpleNode(Node *node, Node *parentNode, int prettyFlags)
72837283
caseT_MinMaxExpr:
72847284
caseT_SQLValueFunction:
72857285
caseT_XmlExpr:
7286+
caseT_NextValueExpr:
72867287
caseT_NullIfExpr:
72877288
caseT_Aggref:
72887289
caseT_WindowFunc:
@@ -8612,6 +8613,22 @@ get_rule_expr(Node *node, deparse_context *context,
86128613
}
86138614
break;
86148615

8616+
caseT_NextValueExpr:
8617+
{
8618+
NextValueExpr*nvexpr= (NextValueExpr*)node;
8619+
8620+
/*
8621+
* This isn't exactly nextval(), but that seems close enough
8622+
* for EXPLAIN's purposes.
8623+
*/
8624+
appendStringInfoString(buf,"nextval(");
8625+
simple_quote_literal(buf,
8626+
generate_relation_name(nvexpr->seqid,
8627+
NIL));
8628+
appendStringInfoChar(buf,')');
8629+
}
8630+
break;
8631+
86158632
caseT_InferenceElem:
86168633
{
86178634
InferenceElem*iexpr= (InferenceElem*)node;

‎src/include/executor/execExpr.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ typedef struct ExprEvalStep
362362
SQLValueFunction*svf;
363363
}sqlvaluefunction;
364364

365-
/* forEEOP_NEXTVALUEXPR */
365+
/* forEEOP_NEXTVALUEEXPR */
366366
struct
367367
{
368368
Oidseqid;
@@ -615,6 +615,7 @@ extern void ExecEvalParamExtern(ExprState *state, ExprEvalStep *op,
615615
ExprContext*econtext);
616616
externvoidExecEvalSQLValueFunction(ExprState*state,ExprEvalStep*op);
617617
externvoidExecEvalCurrentOfExpr(ExprState*state,ExprEvalStep*op);
618+
externvoidExecEvalNextValueExpr(ExprState*state,ExprEvalStep*op);
618619
externvoidExecEvalRowNull(ExprState*state,ExprEvalStep*op,
619620
ExprContext*econtext);
620621
externvoidExecEvalRowNotNull(ExprState*state,ExprEvalStep*op,

‎src/include/nodes/nodes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,14 @@ typedef enum NodeTag
183183
T_CoerceToDomainValue,
184184
T_SetToDefault,
185185
T_CurrentOfExpr,
186+
T_NextValueExpr,
186187
T_InferenceElem,
187188
T_TargetEntry,
188189
T_RangeTblRef,
189190
T_JoinExpr,
190191
T_FromExpr,
191192
T_OnConflictExpr,
192193
T_IntoClause,
193-
T_NextValueExpr,
194194

195195
/*
196196
* TAGS FOR EXPRESSION STATE NODES (execnodes.h)

‎src/include/nodes/primnodes.h

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,6 +1279,20 @@ typedef struct CurrentOfExpr
12791279
intcursor_param;/* refcursor parameter number, or 0 */
12801280
}CurrentOfExpr;
12811281

1282+
/*
1283+
* NextValueExpr - get next value from sequence
1284+
*
1285+
* This has the same effect as calling the nextval() function, but it does not
1286+
* check permissions on the sequence. This is used for identity columns,
1287+
* where the sequence is an implicit dependency without its own permissions.
1288+
*/
1289+
typedefstructNextValueExpr
1290+
{
1291+
Exprxpr;
1292+
Oidseqid;
1293+
OidtypeId;
1294+
}NextValueExpr;
1295+
12821296
/*
12831297
* InferenceElem - an element of a unique index inference specification
12841298
*
@@ -1294,20 +1308,6 @@ typedef struct InferenceElem
12941308
Oidinferopclass;/* OID of att opclass, or InvalidOid */
12951309
}InferenceElem;
12961310

1297-
/*
1298-
* NextValueExpr - get next value from sequence
1299-
*
1300-
* This has the same effect as calling the nextval() function, but it does not
1301-
* check permissions on the sequence. This is used for identity columns,
1302-
* where the sequence is an implicit dependency without its own permissions.
1303-
*/
1304-
typedefstructNextValueExpr
1305-
{
1306-
Exprxpr;
1307-
Oidseqid;
1308-
OidtypeId;
1309-
}NextValueExpr;
1310-
13111311
/*--------------------
13121312
* TargetEntry -
13131313
* a target entry (used in query target lists)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp