diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y index 9edd1a0aff..fdb1f7faac 100644 --- a/src/backend/bootstrap/bootparse.y +++ b/src/backend/bootstrap/bootparse.y @@ -304,7 +304,9 @@ Boot_DeclareIndexStmt: stmt->isconstraint = false; stmt->deferrable = false; stmt->initdeferred = false; + stmt->transformed = false; stmt->concurrent = false; + stmt->if_not_exists = false; /* locks and races need not concern us in bootstrap mode */ relationId = RangeVarGetRelid(stmt->relation, NoLock, @@ -345,7 +347,9 @@ Boot_DeclareUniqueIndexStmt: stmt->isconstraint = false; stmt->deferrable = false; stmt->initdeferred = false; + stmt->transformed = false; stmt->concurrent = false; + stmt->if_not_exists = false; /* locks and races need not concern us in bootstrap mode */ relationId = RangeVarGetRelid(stmt->relation, NoLock, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b2993b80fb..f5d5b6302f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5705,6 +5705,9 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel, Assert(IsA(stmt, IndexStmt)); Assert(!stmt->concurrent); + /* The IndexStmt has already been through transformIndexStmt */ + Assert(stmt->transformed); + /* suppress schema rights check when rebuilding existing index */ check_rights = !is_rebuild; /* skip index build if phase 3 will do it or we're reusing an old one */ @@ -5712,8 +5715,6 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel, /* suppress notices when rebuilding existing index */ quiet = is_rebuild; - /* The IndexStmt has already been through transformIndexStmt */ - new_index = DefineIndex(RelationGetRelid(rel), stmt, InvalidOid, /* no predefined OID */ diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index e5b0dce477..8798cbe43d 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2937,6 +2937,7 @@ _copyIndexStmt(const IndexStmt *from) COPY_SCALAR_FIELD(isconstraint); COPY_SCALAR_FIELD(deferrable); COPY_SCALAR_FIELD(initdeferred); + COPY_SCALAR_FIELD(transformed); COPY_SCALAR_FIELD(concurrent); COPY_SCALAR_FIELD(if_not_exists); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 6e8b308a3e..a90386b800 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1209,6 +1209,7 @@ _equalIndexStmt(const IndexStmt *a, const IndexStmt *b) COMPARE_SCALAR_FIELD(isconstraint); COMPARE_SCALAR_FIELD(deferrable); COMPARE_SCALAR_FIELD(initdeferred); + COMPARE_SCALAR_FIELD(transformed); COMPARE_SCALAR_FIELD(concurrent); COMPARE_SCALAR_FIELD(if_not_exists); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index d8e2077d60..67b7d40406 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2078,7 +2078,9 @@ _outIndexStmt(StringInfo str, const IndexStmt *node) WRITE_BOOL_FIELD(isconstraint); WRITE_BOOL_FIELD(deferrable); WRITE_BOOL_FIELD(initdeferred); + WRITE_BOOL_FIELD(transformed); WRITE_BOOL_FIELD(concurrent); + WRITE_BOOL_FIELD(if_not_exists); } static void diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 67c7c84dee..b8af296397 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -6563,6 +6563,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_index_name n->isconstraint = false; n->deferrable = false; n->initdeferred = false; + n->transformed = false; n->if_not_exists = false; $$ = (Node *)n; } @@ -6588,6 +6589,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_index_name n->isconstraint = false; n->deferrable = false; n->initdeferred = false; + n->transformed = false; n->if_not_exists = true; $$ = (Node *)n; } diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 654dce6755..8d90b5098a 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -339,10 +339,11 @@ transformJoinUsingClause(ParseState *pstate, /* * We cheat a little bit here by building an untransformed operator tree - * whose leaves are the already-transformed Vars. This is OK because - * transformExpr() won't complain about already-transformed subnodes. - * However, this does mean that we have to mark the columns as requiring - * SELECT privilege for ourselves; transformExpr() won't do it. + * whose leaves are the already-transformed Vars. This requires collusion + * from transformExpr(), which normally could be expected to complain + * about already-transformed subnodes. However, this does mean that we + * have to mark the columns as requiring SELECT privilege for ourselves; + * transformExpr() won't do it. */ forboth(lvars, leftVars, rvars, rightVars) { diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 67a2310ad0..791639a7db 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -81,28 +81,8 @@ static Expr *make_distinct_op(ParseState *pstate, List *opname, /* * transformExpr - * Analyze and transform expressions. Type checking and type casting is - * done here. The optimizer and the executor cannot handle the original - * (raw) expressions collected by the parse tree. Hence the transformation - * here. - * - * NOTE: there are various cases in which this routine will get applied to - * an already-transformed expression. Some examples: - * 1. At least one construct (BETWEEN/AND) puts the same nodes - * into two branches of the parse tree; hence, some nodes - * are transformed twice. - * 2. Another way it can happen is that coercion of an operator or - * function argument to the required type (via coerce_type()) - * can apply transformExpr to an already-transformed subexpression. - * An example here is "SELECT count(*) + 1.0 FROM table". - * 3. CREATE TABLE t1 (LIKE t2 INCLUDING INDEXES) can pass in - * already-transformed index expressions. - * While it might be possible to eliminate these cases, the path of - * least resistance so far has been to ensure that transformExpr() does - * no damage if applied to an already-transformed tree. This is pretty - * easy for cases where the transformation replaces one node type with - * another, such as A_Const => Const; we just do nothing when handed - * a Const. More care is needed for node types that are used as both - * input and output of transformExpr; see SubLink for example. + * done here. This processing converts the raw grammar output into + * expression trees with fully determined semantics. */ Node * transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind) @@ -168,48 +148,8 @@ transformExprRecurse(ParseState *pstate, Node *expr) break; case T_TypeCast: - { - TypeCast *tc = (TypeCast *) expr; - - /* - * If the subject of the typecast is an ARRAY[] construct and - * the target type is an array type, we invoke - * transformArrayExpr() directly so that we can pass down the - * type information. This avoids some cases where - * transformArrayExpr() might not infer the correct type. - */ - if (IsA(tc->arg, A_ArrayExpr)) - { - Oid targetType; - Oid elementType; - int32 targetTypmod; - - typenameTypeIdAndMod(pstate, tc->typeName, - &targetType, &targetTypmod); - - /* - * If target is a domain over array, work with the base - * array type here. transformTypeCast below will cast the - * array type to the domain. In the usual case that the - * target is not a domain, transformTypeCast is a no-op. - */ - targetType = getBaseTypeAndTypmod(targetType, - &targetTypmod); - elementType = get_element_type(targetType); - if (OidIsValid(elementType)) - { - tc = copyObject(tc); - tc->arg = transformArrayExpr(pstate, - (A_ArrayExpr *) tc->arg, - targetType, - elementType, - targetTypmod); - } - } - - result = transformTypeCast(pstate, tc); - break; - } + result = transformTypeCast(pstate, (TypeCast *) expr); + break; case T_CollateClause: result = transformCollateClause(pstate, (CollateClause *) expr); @@ -324,37 +264,19 @@ transformExprRecurse(ParseState *pstate, Node *expr) result = transformCurrentOfExpr(pstate, (CurrentOfExpr *) expr); break; - /********************************************* - * Quietly accept node types that may be presented when we are - * called on an already-transformed tree. + /* + * CaseTestExpr and SetToDefault don't require any processing; + * they are only injected into parse trees in fully-formed state. * - * Do any other node types need to be accepted? For now we are - * taking a conservative approach, and only accepting node - * types that are demonstrably necessary to accept. - *********************************************/ - case T_Var: - case T_Const: - case T_Param: - case T_Aggref: - case T_WindowFunc: - case T_ArrayRef: - case T_FuncExpr: - case T_OpExpr: - case T_DistinctExpr: - case T_NullIfExpr: - case T_ScalarArrayOpExpr: - case T_FieldSelect: - case T_FieldStore: - case T_RelabelType: - case T_CoerceViaIO: - case T_ArrayCoerceExpr: - case T_ConvertRowtypeExpr: - case T_CollateExpr: + * Ordinarily we should not see a Var here, but it is convenient + * for transformJoinUsingClause() to create untransformed operator + * trees containing already-transformed Vars. The best + * alternative would be to deconstruct and reconstruct column + * references, which seems expensively pointless. So allow it. + */ case T_CaseTestExpr: - case T_ArrayExpr: - case T_CoerceToDomain: - case T_CoerceToDomainValue: case T_SetToDefault: + case T_Var: { result = (Node *) expr; break; @@ -1461,10 +1383,6 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c) Node *defresult; Oid ptype; - /* If we already transformed this node, do nothing */ - if (OidIsValid(c->casetype)) - return (Node *) c; - newc = makeNode(CaseExpr); /* transform the test expression, if any */ @@ -1592,10 +1510,6 @@ transformSubLink(ParseState *pstate, SubLink *sublink) Query *qtree; const char *err; - /* If we already transformed this node, do nothing */ - if (IsA(sublink->subselect, Query)) - return result; - /* * Check to see if the sublink is in an invalid place within the query. We * allow sublinks everywhere in SELECT/INSERT/UPDATE/DELETE, but generally @@ -1964,10 +1878,6 @@ transformRowExpr(ParseState *pstate, RowExpr *r) int fnum; ListCell *lc; - /* If we already transformed this node, do nothing */ - if (OidIsValid(r->row_typeid)) - return (Node *) r; - newr = makeNode(RowExpr); /* Transform the field expressions */ @@ -2074,10 +1984,6 @@ transformXmlExpr(ParseState *pstate, XmlExpr *x) ListCell *lc; int i; - /* If we already transformed this node, do nothing */ - if (OidIsValid(x->type)) - return (Node *) x; - newx = makeNode(XmlExpr); newx->op = x->op; if (x->name) @@ -2381,14 +2287,51 @@ static Node * transformTypeCast(ParseState *pstate, TypeCast *tc) { Node *result; - Node *expr = transformExprRecurse(pstate, tc->arg); - Oid inputType = exprType(expr); + Node *expr; + Oid inputType; Oid targetType; int32 targetTypmod; int location; typenameTypeIdAndMod(pstate, tc->typeName, &targetType, &targetTypmod); + /* + * If the subject of the typecast is an ARRAY[] construct and the target + * type is an array type, we invoke transformArrayExpr() directly so that + * we can pass down the type information. This avoids some cases where + * transformArrayExpr() might not infer the correct type. Otherwise, just + * transform the argument normally. + */ + if (IsA(tc->arg, A_ArrayExpr)) + { + Oid targetBaseType; + int32 targetBaseTypmod; + Oid elementType; + + /* + * If target is a domain over array, work with the base array type + * here. Below, we'll cast the array type to the domain. In the + * usual case that the target is not a domain, the remaining steps + * will be a no-op. + */ + targetBaseTypmod = targetTypmod; + targetBaseType = getBaseTypeAndTypmod(targetType, &targetBaseTypmod); + elementType = get_element_type(targetBaseType); + if (OidIsValid(elementType)) + { + expr = transformArrayExpr(pstate, + (A_ArrayExpr *) tc->arg, + targetBaseType, + elementType, + targetBaseTypmod); + } + else + expr = transformExprRecurse(pstate, tc->arg); + } + else + expr = transformExprRecurse(pstate, tc->arg); + + inputType = exprType(expr); if (inputType == InvalidOid) return expr; /* do nothing if NULL input */ diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 7540043ce5..c29f106529 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -1072,7 +1072,9 @@ generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx, index->oldNode = InvalidOid; index->unique = idxrec->indisunique; index->primary = idxrec->indisprimary; + index->transformed = true; /* don't need transformIndexStmt */ index->concurrent = false; + index->if_not_exists = false; /* * We don't try to preserve the name of the source index; instead, just @@ -1530,7 +1532,9 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) index->idxcomment = NULL; index->indexOid = InvalidOid; index->oldNode = InvalidOid; + index->transformed = false; index->concurrent = false; + index->if_not_exists = false; /* * 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) ListCell *l; Relation rel; + /* Nothing to do if statement already transformed. */ + if (stmt->transformed) + return stmt; + /* * We must not scribble on the passed-in IndexStmt, so copy it. (This is * overkill, but easy.) @@ -2021,6 +2029,9 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString) /* Close relation */ heap_close(rel, NoLock); + /* Mark statement as successfully transformed */ + stmt->transformed = true; + return stmt; } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 35b68ec033..d7b6148cd5 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2261,6 +2261,7 @@ typedef struct IndexStmt bool isconstraint; /* is it for a pkey/unique constraint? */ bool deferrable; /* is the constraint DEFERRABLE? */ bool initdeferred; /* is the constraint INITIALLY DEFERRED? */ + bool transformed; /* true when transformIndexStmt is finished */ bool concurrent; /* should this be a concurrent index build? */ bool if_not_exists; /* just do nothing if index already exists? */ } IndexStmt;