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

Commit6a75562

Browse files
committed
Get rid of multiple applications of transformExpr() to the same tree.
transformExpr() has for many years had provisions to do nothing whenapplied to an already-transformed expression tree. However, this wasalways ugly and of dubious reliability, so we'd be much better off withoutit. The primary historical reason for it was that gram.y sometimesreturned multiple links to the same subexpression, which is no longer trueas of my BETWEEN fixes. We'd also grown some lazy hacks in CREATE TABLELIKE (failing to distinguish between raw and already-transformed indexspecifications) and one or two other places.This patch removes the need for and support for re-transforming alreadytransformed expressions. The index case is dealt with by adding a flagto struct IndexStmt to indicate that it's already been transformed;which has some benefit anyway in that tablecmds.c can now Assert thattransformation has happened rather than just assuming. The other mainreason was some rather sloppy code for array type coercion, which canbe fixed (and its performance improved too) by refactoring.I did leave transformJoinUsingClause() still constructing expressionscontaining untransformed operator nodes being applied to Vars, so thattransformExpr() still has to allow Var inputs. But that's a much narrower,and safer, special case than before, since Vars will never appear in a rawparse tree, and they don't have any substructure to worry about.In passing fix some oversights in the patch that added CREATE INDEXIF NOT EXISTS (missing processing of IndexStmt.if_not_exists). Theseappear relatively harmless, but still sloppy coding practice.
1 parent34af082 commit6a75562

File tree

10 files changed

+83
-116
lines changed

10 files changed

+83
-116
lines changed

‎src/backend/bootstrap/bootparse.y

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,9 @@ Boot_DeclareIndexStmt:
304304
stmt->isconstraint =false;
305305
stmt->deferrable =false;
306306
stmt->initdeferred =false;
307+
stmt->transformed =false;
307308
stmt->concurrent =false;
309+
stmt->if_not_exists =false;
308310

309311
/* locks and races need not concern us in bootstrap mode*/
310312
relationId = RangeVarGetRelid(stmt->relation, NoLock,
@@ -345,7 +347,9 @@ Boot_DeclareUniqueIndexStmt:
345347
stmt->isconstraint =false;
346348
stmt->deferrable =false;
347349
stmt->initdeferred =false;
350+
stmt->transformed =false;
348351
stmt->concurrent =false;
352+
stmt->if_not_exists =false;
349353

350354
/* locks and races need not concern us in bootstrap mode*/
351355
relationId = RangeVarGetRelid(stmt->relation, NoLock,

‎src/backend/commands/tablecmds.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5705,15 +5705,16 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
57055705
Assert(IsA(stmt,IndexStmt));
57065706
Assert(!stmt->concurrent);
57075707

5708+
/* The IndexStmt has already been through transformIndexStmt */
5709+
Assert(stmt->transformed);
5710+
57085711
/* suppress schema rights check when rebuilding existing index */
57095712
check_rights= !is_rebuild;
57105713
/* skip index build if phase 3 will do it or we're reusing an old one */
57115714
skip_build=tab->rewrite>0||OidIsValid(stmt->oldNode);
57125715
/* suppress notices when rebuilding existing index */
57135716
quiet=is_rebuild;
57145717

5715-
/* The IndexStmt has already been through transformIndexStmt */
5716-
57175718
new_index=DefineIndex(RelationGetRelid(rel),
57185719
stmt,
57195720
InvalidOid,/* no predefined OID */

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2937,6 +2937,7 @@ _copyIndexStmt(const IndexStmt *from)
29372937
COPY_SCALAR_FIELD(isconstraint);
29382938
COPY_SCALAR_FIELD(deferrable);
29392939
COPY_SCALAR_FIELD(initdeferred);
2940+
COPY_SCALAR_FIELD(transformed);
29402941
COPY_SCALAR_FIELD(concurrent);
29412942
COPY_SCALAR_FIELD(if_not_exists);
29422943

‎src/backend/nodes/equalfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,7 @@ _equalIndexStmt(const IndexStmt *a, const IndexStmt *b)
12091209
COMPARE_SCALAR_FIELD(isconstraint);
12101210
COMPARE_SCALAR_FIELD(deferrable);
12111211
COMPARE_SCALAR_FIELD(initdeferred);
1212+
COMPARE_SCALAR_FIELD(transformed);
12121213
COMPARE_SCALAR_FIELD(concurrent);
12131214
COMPARE_SCALAR_FIELD(if_not_exists);
12141215

‎src/backend/nodes/outfuncs.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2078,7 +2078,9 @@ _outIndexStmt(StringInfo str, const IndexStmt *node)
20782078
WRITE_BOOL_FIELD(isconstraint);
20792079
WRITE_BOOL_FIELD(deferrable);
20802080
WRITE_BOOL_FIELD(initdeferred);
2081+
WRITE_BOOL_FIELD(transformed);
20812082
WRITE_BOOL_FIELD(concurrent);
2083+
WRITE_BOOL_FIELD(if_not_exists);
20822084
}
20832085

20842086
staticvoid

‎src/backend/parser/gram.y

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6563,6 +6563,7 @@ IndexStmt:CREATE opt_unique INDEX opt_concurrently opt_index_name
65636563
n->isconstraint =false;
65646564
n->deferrable =false;
65656565
n->initdeferred =false;
6566+
n->transformed =false;
65666567
n->if_not_exists =false;
65676568
$$ = (Node *)n;
65686569
}
@@ -6588,6 +6589,7 @@ IndexStmt:CREATE opt_unique INDEX opt_concurrently opt_index_name
65886589
n->isconstraint =false;
65896590
n->deferrable =false;
65906591
n->initdeferred =false;
6592+
n->transformed =false;
65916593
n->if_not_exists =true;
65926594
$$ = (Node *)n;
65936595
}

‎src/backend/parser/parse_clause.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -339,10 +339,11 @@ transformJoinUsingClause(ParseState *pstate,
339339

340340
/*
341341
* We cheat a little bit here by building an untransformed operator tree
342-
* whose leaves are the already-transformed Vars. This is OK because
343-
* transformExpr() won't complain about already-transformed subnodes.
344-
* However, this does mean that we have to mark the columns as requiring
345-
* SELECT privilege for ourselves; transformExpr() won't do it.
342+
* whose leaves are the already-transformed Vars. This requires collusion
343+
* from transformExpr(), which normally could be expected to complain
344+
* about already-transformed subnodes. However, this does mean that we
345+
* have to mark the columns as requiring SELECT privilege for ourselves;
346+
* transformExpr() won't do it.
346347
*/
347348
forboth(lvars,leftVars,rvars,rightVars)
348349
{

‎src/backend/parser/parse_expr.c

Lines changed: 53 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -81,28 +81,8 @@ static Expr *make_distinct_op(ParseState *pstate, List *opname,
8181
/*
8282
* transformExpr -
8383
* Analyze and transform expressions. Type checking and type casting is
84-
* done here. The optimizer and the executor cannot handle the original
85-
* (raw) expressions collected by the parse tree. Hence the transformation
86-
* here.
87-
*
88-
* NOTE: there are various cases in which this routine will get applied to
89-
* an already-transformed expression. Some examples:
90-
*1. At least one construct (BETWEEN/AND) puts the same nodes
91-
*into two branches of the parse tree; hence, some nodes
92-
*are transformed twice.
93-
*2. Another way it can happen is that coercion of an operator or
94-
*function argument to the required type (via coerce_type())
95-
*can apply transformExpr to an already-transformed subexpression.
96-
*An example here is "SELECT count(*) + 1.0 FROM table".
97-
*3. CREATE TABLE t1 (LIKE t2 INCLUDING INDEXES) can pass in
98-
*already-transformed index expressions.
99-
* While it might be possible to eliminate these cases, the path of
100-
* least resistance so far has been to ensure that transformExpr() does
101-
* no damage if applied to an already-transformed tree. This is pretty
102-
* easy for cases where the transformation replaces one node type with
103-
* another, such as A_Const => Const; we just do nothing when handed
104-
* a Const. More care is needed for node types that are used as both
105-
* input and output of transformExpr; see SubLink for example.
84+
* done here. This processing converts the raw grammar output into
85+
* expression trees with fully determined semantics.
10686
*/
10787
Node*
10888
transformExpr(ParseState*pstate,Node*expr,ParseExprKindexprKind)
@@ -168,48 +148,8 @@ transformExprRecurse(ParseState *pstate, Node *expr)
168148
break;
169149

170150
caseT_TypeCast:
171-
{
172-
TypeCast*tc= (TypeCast*)expr;
173-
174-
/*
175-
* If the subject of the typecast is an ARRAY[] construct and
176-
* the target type is an array type, we invoke
177-
* transformArrayExpr() directly so that we can pass down the
178-
* type information. This avoids some cases where
179-
* transformArrayExpr() might not infer the correct type.
180-
*/
181-
if (IsA(tc->arg,A_ArrayExpr))
182-
{
183-
OidtargetType;
184-
OidelementType;
185-
int32targetTypmod;
186-
187-
typenameTypeIdAndMod(pstate,tc->typeName,
188-
&targetType,&targetTypmod);
189-
190-
/*
191-
* If target is a domain over array, work with the base
192-
* array type here. transformTypeCast below will cast the
193-
* array type to the domain. In the usual case that the
194-
* target is not a domain, transformTypeCast is a no-op.
195-
*/
196-
targetType=getBaseTypeAndTypmod(targetType,
197-
&targetTypmod);
198-
elementType=get_element_type(targetType);
199-
if (OidIsValid(elementType))
200-
{
201-
tc=copyObject(tc);
202-
tc->arg=transformArrayExpr(pstate,
203-
(A_ArrayExpr*)tc->arg,
204-
targetType,
205-
elementType,
206-
targetTypmod);
207-
}
208-
}
209-
210-
result=transformTypeCast(pstate,tc);
211-
break;
212-
}
151+
result=transformTypeCast(pstate, (TypeCast*)expr);
152+
break;
213153

214154
caseT_CollateClause:
215155
result=transformCollateClause(pstate, (CollateClause*)expr);
@@ -324,37 +264,19 @@ transformExprRecurse(ParseState *pstate, Node *expr)
324264
result=transformCurrentOfExpr(pstate, (CurrentOfExpr*)expr);
325265
break;
326266

327-
/*********************************************
328-
*Quietly accept node types that may be presented when we are
329-
*called on an already-transformed tree.
267+
/*
268+
*CaseTestExpr and SetToDefault don't require any processing;
269+
*they are only injected into parse trees in fully-formed state.
330270
*
331-
* Do any other node types need to be accepted? For now we are
332-
* taking a conservative approach, and only accepting node
333-
* types that are demonstrably necessary to accept.
334-
*********************************************/
335-
caseT_Var:
336-
caseT_Const:
337-
caseT_Param:
338-
caseT_Aggref:
339-
caseT_WindowFunc:
340-
caseT_ArrayRef:
341-
caseT_FuncExpr:
342-
caseT_OpExpr:
343-
caseT_DistinctExpr:
344-
caseT_NullIfExpr:
345-
caseT_ScalarArrayOpExpr:
346-
caseT_FieldSelect:
347-
caseT_FieldStore:
348-
caseT_RelabelType:
349-
caseT_CoerceViaIO:
350-
caseT_ArrayCoerceExpr:
351-
caseT_ConvertRowtypeExpr:
352-
caseT_CollateExpr:
271+
* Ordinarily we should not see a Var here, but it is convenient
272+
* for transformJoinUsingClause() to create untransformed operator
273+
* trees containing already-transformed Vars. The best
274+
* alternative would be to deconstruct and reconstruct column
275+
* references, which seems expensively pointless. So allow it.
276+
*/
353277
caseT_CaseTestExpr:
354-
caseT_ArrayExpr:
355-
caseT_CoerceToDomain:
356-
caseT_CoerceToDomainValue:
357278
caseT_SetToDefault:
279+
caseT_Var:
358280
{
359281
result= (Node*)expr;
360282
break;
@@ -1461,10 +1383,6 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c)
14611383
Node*defresult;
14621384
Oidptype;
14631385

1464-
/* If we already transformed this node, do nothing */
1465-
if (OidIsValid(c->casetype))
1466-
return (Node*)c;
1467-
14681386
newc=makeNode(CaseExpr);
14691387

14701388
/* transform the test expression, if any */
@@ -1592,10 +1510,6 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
15921510
Query*qtree;
15931511
constchar*err;
15941512

1595-
/* If we already transformed this node, do nothing */
1596-
if (IsA(sublink->subselect,Query))
1597-
returnresult;
1598-
15991513
/*
16001514
* Check to see if the sublink is in an invalid place within the query. We
16011515
* allow sublinks everywhere in SELECT/INSERT/UPDATE/DELETE, but generally
@@ -1964,10 +1878,6 @@ transformRowExpr(ParseState *pstate, RowExpr *r)
19641878
intfnum;
19651879
ListCell*lc;
19661880

1967-
/* If we already transformed this node, do nothing */
1968-
if (OidIsValid(r->row_typeid))
1969-
return (Node*)r;
1970-
19711881
newr=makeNode(RowExpr);
19721882

19731883
/* Transform the field expressions */
@@ -2074,10 +1984,6 @@ transformXmlExpr(ParseState *pstate, XmlExpr *x)
20741984
ListCell*lc;
20751985
inti;
20761986

2077-
/* If we already transformed this node, do nothing */
2078-
if (OidIsValid(x->type))
2079-
return (Node*)x;
2080-
20811987
newx=makeNode(XmlExpr);
20821988
newx->op=x->op;
20831989
if (x->name)
@@ -2381,14 +2287,51 @@ static Node *
23812287
transformTypeCast(ParseState*pstate,TypeCast*tc)
23822288
{
23832289
Node*result;
2384-
Node*expr=transformExprRecurse(pstate,tc->arg);
2385-
OidinputType=exprType(expr);
2290+
Node*expr;
2291+
OidinputType;
23862292
OidtargetType;
23872293
int32targetTypmod;
23882294
intlocation;
23892295

23902296
typenameTypeIdAndMod(pstate,tc->typeName,&targetType,&targetTypmod);
23912297

2298+
/*
2299+
* If the subject of the typecast is an ARRAY[] construct and the target
2300+
* type is an array type, we invoke transformArrayExpr() directly so that
2301+
* we can pass down the type information. This avoids some cases where
2302+
* transformArrayExpr() might not infer the correct type. Otherwise, just
2303+
* transform the argument normally.
2304+
*/
2305+
if (IsA(tc->arg,A_ArrayExpr))
2306+
{
2307+
OidtargetBaseType;
2308+
int32targetBaseTypmod;
2309+
OidelementType;
2310+
2311+
/*
2312+
* If target is a domain over array, work with the base array type
2313+
* here. Below, we'll cast the array type to the domain. In the
2314+
* usual case that the target is not a domain, the remaining steps
2315+
* will be a no-op.
2316+
*/
2317+
targetBaseTypmod=targetTypmod;
2318+
targetBaseType=getBaseTypeAndTypmod(targetType,&targetBaseTypmod);
2319+
elementType=get_element_type(targetBaseType);
2320+
if (OidIsValid(elementType))
2321+
{
2322+
expr=transformArrayExpr(pstate,
2323+
(A_ArrayExpr*)tc->arg,
2324+
targetBaseType,
2325+
elementType,
2326+
targetBaseTypmod);
2327+
}
2328+
else
2329+
expr=transformExprRecurse(pstate,tc->arg);
2330+
}
2331+
else
2332+
expr=transformExprRecurse(pstate,tc->arg);
2333+
2334+
inputType=exprType(expr);
23922335
if (inputType==InvalidOid)
23932336
returnexpr;/* do nothing if NULL input */
23942337

‎src/backend/parser/parse_utilcmd.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,9 @@ generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx,
10721072
index->oldNode=InvalidOid;
10731073
index->unique=idxrec->indisunique;
10741074
index->primary=idxrec->indisprimary;
1075+
index->transformed= true;/* don't need transformIndexStmt */
10751076
index->concurrent= false;
1077+
index->if_not_exists= false;
10761078

10771079
/*
10781080
* We don't try to preserve the name of the source index; instead, just
@@ -1530,7 +1532,9 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
15301532
index->idxcomment=NULL;
15311533
index->indexOid=InvalidOid;
15321534
index->oldNode=InvalidOid;
1535+
index->transformed= false;
15331536
index->concurrent= false;
1537+
index->if_not_exists= false;
15341538

15351539
/*
15361540
* If it's ALTER TABLE ADD CONSTRAINT USING INDEX, look up the index and
@@ -1941,6 +1945,10 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString)
19411945
ListCell*l;
19421946
Relationrel;
19431947

1948+
/* Nothing to do if statement already transformed. */
1949+
if (stmt->transformed)
1950+
returnstmt;
1951+
19441952
/*
19451953
* We must not scribble on the passed-in IndexStmt, so copy it. (This is
19461954
* overkill, but easy.)
@@ -2021,6 +2029,9 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString)
20212029
/* Close relation */
20222030
heap_close(rel,NoLock);
20232031

2032+
/* Mark statement as successfully transformed */
2033+
stmt->transformed= true;
2034+
20242035
returnstmt;
20252036
}
20262037

‎src/include/nodes/parsenodes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2261,6 +2261,7 @@ typedef struct IndexStmt
22612261
boolisconstraint;/* is it for a pkey/unique constraint? */
22622262
booldeferrable;/* is the constraint DEFERRABLE? */
22632263
boolinitdeferred;/* is the constraint INITIALLY DEFERRED? */
2264+
booltransformed;/* true when transformIndexStmt is finished */
22642265
boolconcurrent;/* should this be a concurrent index build? */
22652266
boolif_not_exists;/* just do nothing if index already exists? */
22662267
}IndexStmt;

0 commit comments

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