at2v5-domains.patch

text/plain

Filename: at2v5-domains.patch
Type: text/plain
Part: 0
Message: Re: ALTER TYPE 2: skip already-provable no-work rewrites

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
File+
src/backend/catalog/index.c 0 0
srindex 3b99e86..0420922 100644 0 0
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 452ced6..466be25 100644
diff --git a/src/backend/commands/index 1db42d0..2e10661 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 142,147 **** typedef struct AlteredTableInfo
--- 142,148 ----
  	List	   *newvals;		/* List of NewColumnValue */
  	bool		new_notnull;	/* T if we added new NOT NULL constraints */
  	bool		rewrite;		/* T if we a rewrite is forced */
+ 	bool		verify;			/* T if we shall verify constraints */
  	Oid			newTableSpace;	/* new tablespace; 0 means no change */
  	/* Objects to rebuild after completing ALTER TYPE operations */
  	List	   *changedConstraintOids;	/* OIDs of constraints to rebuild */
***************
*** 336,342 **** static void ATPrepAlterColumnType(List **wqueue,
  					  AlteredTableInfo *tab, Relation rel,
  					  bool recurse, bool recursing,
  					  AlterTableCmd *cmd, LOCKMODE lockmode);
- static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno);
  static void ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
  					  const char *colName, TypeName *typeName, LOCKMODE lockmode);
  static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode);
--- 337,342 ----
***************
*** 3242,3247 **** ATRewriteTables(List **wqueue, LOCKMODE lockmode)
--- 3242,3251 ----
  			heap_close(rel, NoLock);
  		}
  
+ 		/* New NOT NULL constraints always require a scan. */
+ 		if (tab->new_notnull)
+ 			tab->verify = true;
+ 
  		/*
  		 * We only need to rewrite the table if at least one column needs to
  		 * be recomputed, or we are adding/removing the OID column.
***************
*** 3313,3319 **** ATRewriteTables(List **wqueue, LOCKMODE lockmode)
  			 * Test the current data within the table against new constraints
  			 * generated by ALTER TABLE commands, but don't rebuild data.
  			 */
! 			if (tab->constraints != NIL || tab->new_notnull)
  				ATRewriteTable(tab, InvalidOid, lockmode);
  
  			/*
--- 3317,3323 ----
  			 * Test the current data within the table against new constraints
  			 * generated by ALTER TABLE commands, but don't rebuild data.
  			 */
! 			if (tab->verify)
  				ATRewriteTable(tab, InvalidOid, lockmode);
  
  			/*
***************
*** 3387,3393 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
  	Relation	newrel;
  	TupleDesc	oldTupDesc;
  	TupleDesc	newTupDesc;
- 	bool		needscan = false;
  	List	   *notnull_attrs;
  	int			i;
  	ListCell   *l;
--- 3391,3396 ----
***************
*** 3446,3452 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
  		switch (con->contype)
  		{
  			case CONSTR_CHECK:
- 				needscan = true;
  				con->qualstate = (List *)
  					ExecPrepareExpr((Expr *) con->qual, estate);
  				break;
--- 3449,3454 ----
***************
*** 3481,3491 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
  				!newTupDesc->attrs[i]->attisdropped)
  				notnull_attrs = lappend_int(notnull_attrs, i);
  		}
- 		if (notnull_attrs)
- 			needscan = true;
  	}
  
- 	if (newrel || needscan)
  	{
  		ExprContext *econtext;
  		Datum	   *values;
--- 3483,3490 ----
***************
*** 3549,3558 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
  
  		while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
  		{
! 			if (tab->rewrite)
! 			{
! 				Oid			tupOid = InvalidOid;
  
  				/* Extract data from old tuple */
  				heap_deform_tuple(tuple, oldTupDesc, values, isnull);
  				if (oldTupDesc->tdhasoid)
--- 3548,3557 ----
  
  		while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
  		{
! 			Oid			tupOid = InvalidOid;
  
+ 			if (newrel)
+ 			{
  				/* Extract data from old tuple */
  				heap_deform_tuple(tuple, oldTupDesc, values, isnull);
  				if (oldTupDesc->tdhasoid)
***************
*** 3561,3570 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
  				/* Set dropped attributes to null in new tuple */
  				foreach(lc, dropped_attrs)
  					isnull[lfirst_int(lc)] = true;
  
  				/*
  				 * Process supplied expressions to replace selected columns.
! 				 * Expression inputs come from the old tuple.
  				 */
  				ExecStoreTuple(tuple, oldslot, InvalidBuffer, false);
  				econtext->ecxt_scantuple = oldslot;
--- 3560,3574 ----
  				/* Set dropped attributes to null in new tuple */
  				foreach(lc, dropped_attrs)
  					isnull[lfirst_int(lc)] = true;
+ 			}
  
+ 			if (tab->newvals != NIL)
+ 			{
  				/*
  				 * Process supplied expressions to replace selected columns.
! 				 * Expression inputs come from the old tuple.  If !newrel, we've
! 				 * proven that no expression will change a tuple value, but they
! 				 * may throw errors.
  				 */
  				ExecStoreTuple(tuple, oldslot, InvalidBuffer, false);
  				econtext->ecxt_scantuple = oldslot;
***************
*** 3578,3584 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
--- 3582,3591 ----
  													 &isnull[ex->attnum - 1],
  														  NULL);
  				}
+ 			}
  
+ 			if (newrel)
+ 			{
  				/*
  				 * Form the new tuple. Note that we don't explicitly pfree it,
  				 * since the per-tuple memory context will be reset shortly.
***************
*** 5270,5275 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
--- 5277,5283 ----
  		newcon->qual = (Node *) make_ands_implicit((Expr *) ccon->expr);
  
  		tab->constraints = lappend(tab->constraints, newcon);
+ 		tab->verify = true;
  
  		/* Save the actually assigned name if it was defaulted */
  		if (constr->conname == NULL)
***************
*** 5618,5623 **** ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
--- 5626,5635 ----
  		newcon->qual = (Node *) fkconstraint;
  
  		tab->constraints = lappend(tab->constraints, newcon);
+ 		/*
+ 		 * No need to set tab->verify; foreign key validation is a distinct
+ 		 * aspect of Phase 3.
+ 		 */
  	}
  
  	/*
***************
*** 6581,6588 **** ATPrepAlterColumnType(List **wqueue,
  		newval->expr = (Expr *) transform;
  
  		tab->newvals = lappend(tab->newvals, newval);
! 		if (ATColumnChangeRequiresRewrite(transform, attnum))
! 			tab->rewrite = true;
  	}
  	else if (tab->relkind == RELKIND_FOREIGN_TABLE)
  	{
--- 6593,6625 ----
  		newval->expr = (Expr *) transform;
  
  		tab->newvals = lappend(tab->newvals, newval);
! 
! 		/*
! 		 * Decide how much phase-3 per-tuple work this change entails.  None
! 		 * needed when the present type changes to itself, to a constraint-free
! 		 * domain thereon, or to a type to which it is binary coercible.  Any
! 		 * constrained domain requires a verification scan.  Anything else
! 		 * requires a rewrite.  Beware of a USING clause trying to substitute
! 		 * some other value.
! 		 */
! 		while (!tab->rewrite)
! 		{
! 			/* only one varno, so no need to check that */
! 			if (IsA(transform, Var) && ((Var *) transform)->varattno == attnum)
! 				break;
! 			else if (IsA(transform, RelabelType))
! 				transform = (Node *) ((RelabelType *) transform)->arg;
! 			else if (IsA(transform, CoerceToDomain))
! 			{
! 				CoerceToDomain *d = (CoerceToDomain *) transform;
! 
! 				if (GetDomainConstraints(d->resulttype) != NIL)
! 					tab->verify = true;
! 				transform = (Node *) d->arg;
! 			}
! 			else
! 				tab->rewrite = true;
! 		}
  	}
  	else if (tab->relkind == RELKIND_FOREIGN_TABLE)
  	{
***************
*** 6622,6650 **** ATPrepAlterColumnType(List **wqueue,
  		ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
  }
  
- /*
-  * When the data type of a column is changed, a rewrite might not be require
-  * if the data type is being changed to its current type, or more interestingly
-  * to a type to which it is binary coercible.  But we must check carefully that
-  * the USING clause isn't trying to insert some other value.
-  */
- static bool
- ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno)
- {
- 	Assert(expr != NULL);
- 
- 	for (;;)
- 	{
- 		/* only one varno, so no need to check that */
- 		if (IsA(expr, Var) && ((Var *) expr)->varattno == varattno)
- 			return false;
- 		else if (IsA(expr, RelabelType))
- 			expr = (Node *) ((RelabelType *) expr)->arg;
- 		else
- 			return true;
- 	}
- }
- 
  static void
  ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
  					  const char *colName, TypeName *typeName, LOCKMODE lockmode)
--- 6659,6664 ----
diff --git a/src/test/regress/GNUmakefiindex 4c8af0f..eeb9487 100644
diff --git a/src/test/regress/expecnew file mode 100644
index 0000000..248fb39
diff --git a/src/test/regress/parallel_schedule b/srindex 3b99e86..0420922 100644
diff --git a/src/test/regress/serial_scheindex b348f0e..9954cf7 100644
diff --git a/src/test/regress/sql/big_anew file mode 100644
index 0000000..8302d87