at2v7-domains.patch

text/plain

Filename: at2v7-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..ab0bcda 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 71,76 ****
--- 71,77 ----
  #include "storage/smgr.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
+ #include "utils/datum.h"
  #include "utils/fmgroids.h"
  #include "utils/inval.h"
  #include "utils/lsyscache.h"
***************
*** 142,147 **** typedef struct AlteredTableInfo
--- 143,149 ----
  	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);
--- 338,345 ----
  					  AlteredTableInfo *tab, Relation rel,
  					  bool recurse, bool recursing,
  					  AlterTableCmd *cmd, LOCKMODE lockmode);
! static void ATColumnChangeSetWorkLevel(AlteredTableInfo *tab,
! 						   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);
***************
*** 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);
  
  			/*
--- 3316,3322 ----
  			 * 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;
--- 3390,3395 ----
***************
*** 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;
--- 3448,3453 ----
***************
*** 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;
--- 3482,3489 ----
***************
*** 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)
--- 3547,3560 ----
  
  		while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
  		{
! 			Oid			tupOid = InvalidOid;
  
+ 			if (newrel
+ #ifdef USE_ASSERT_CHECKING
+ 				|| assert_enabled
+ #endif
+ 				)
+ 			{
  				/* 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;
--- 3563,3577 ----
  				/* 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;
***************
*** 3577,3584 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
--- 3584,3618 ----
  														  econtext,
  													 &isnull[ex->attnum - 1],
  														  NULL);
+ 
+ #ifdef USE_ASSERT_CHECKING
+ 					if (assert_enabled)
+ 					{
+ 						Datum		oldval = values[ex->attnum - 1];
+ 						bool		oldisnull = isnull[ex->attnum - 1];
+ 						Form_pg_attribute f = newTupDesc->attrs[ex->attnum - 1];
+ 
+ 						if (f->attbyval && f->attlen == -1)
+ 							oldval = PointerGetDatum(PG_DETOAST_DATUM(oldval));
+ 
+ 						/*
+ 						 * We don't detect the gross error of !newrel when the
+ 						 * typlen actually changed.  attbyval could differ in
+ 						 * theory, but we assume it does not.
+ 						 */
+ 						Assert(newrel ||
+ 							   (isnull[ex->attnum - 1] == oldisnull
+ 								&& (oldisnull ||
+ 									datumIsEqual(oldval,
+ 												 values[ex->attnum - 1],
+ 												 f->attbyval, f->attlen))));
+ 					}
+ #endif
  				}
+ 			}
  
+ 			if (newrel)
+ 			{
  				/*
  				 * Form the new tuple. Note that we don't explicitly pfree it,
  				 * since the per-tuple memory context will be reset shortly.
***************
*** 4357,4362 **** ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
--- 4391,4397 ----
  		 * anything.)
  		 */
  		tab->new_notnull |= colDef->is_not_null;
+ 		tab->verify |= tab->new_notnull;
  	}
  
  	/*
***************
*** 4553,4558 **** ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
--- 4588,4594 ----
  
  		/* Tell Phase 3 it needs to test the constraint */
  		tab->new_notnull = true;
+ 		tab->verify = true;
  	}
  
  	heap_close(attr_rel, RowExclusiveLock);
***************
*** 5270,5275 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
--- 5306,5312 ----
  		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,
--- 5655,5664 ----
  		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)
  	{
--- 6622,6628 ----
  		newval->expr = (Expr *) transform;
  
  		tab->newvals = lappend(tab->newvals, newval);
! 		ATColumnChangeSetWorkLevel(tab, transform, attnum);
  	}
  	else if (tab->relkind == RELKIND_FOREIGN_TABLE)
  	{
***************
*** 6623,6647 **** ATPrepAlterColumnType(List **wqueue,
  }
  
  /*
!  * 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;
  	}
  }
  
--- 6663,6722 ----
  }
  
  /*
!  * Decide how much phase-3 per-tuple work this column type change entails, based
!  * on its transformation expression tree.  There are three possibilities:
!  *
!  *  1. Rewrite; always sufficient.  Example: int2 -> int4.
!  *  2. Verification scan.  Example: text -> constrained domain over text.
!  *  3. No per-tuple work.  Example: varchar(N) -> text.
!  *
!  * To certify #2 or #3 as sufficient, the transformation expression tree must
!  * consist solely of nodes from this list:
!  *
!  *	A. Var with the varattno of the column under alteration.  Allows #2 or #3.
!  *	B. RelabelType.  Allows #2 or #3.
!  *	C. CoerceToDomain.  Allows #2; allows #3 when GetDomainConstraints() == NIL.
!  *
!  * (A) primordially conveys the current column data.  (B) and (C) are acceptable
!  * because they always pass a datum through unchanged; any other node having
!  * that property could potentially join this list.
   */
! static void
! ATColumnChangeSetWorkLevel(AlteredTableInfo *tab,
! 						   Node *expr, AttrNumber varattno)
  {
  	Assert(expr != NULL);
  
! 	/*
! 	 * Walking an expression tree is normally a recursive operation.  Since all
! 	 * node types on our current whitelist have no more than one descendent node
! 	 * requiring assessment, the recursion degenerates to iteration.
! 	 *
! 	 * When tab->rewrite becomes true, no optimizations are possible, and we're
! 	 * done.  It may be set already from analysis of an earlier subcommand in
! 	 * the same ALTER TABLE operation, and that's fine.
! 	 */
! 	while (!tab->rewrite)
  	{
! 		/*
! 		 * Non-matching varattno arises when an explicit USING clause references
! 		 * a different column.  Only one varno, so no need to check that.  When
! 		 * we reach this node, we're done, and some optimizations do apply.
! 		 */
  		if (IsA(expr, Var) && ((Var *) expr)->varattno == varattno)
! 			break;
  		else if (IsA(expr, RelabelType))
  			expr = (Node *) ((RelabelType *) expr)->arg;
+ 		else if (IsA(expr, CoerceToDomain))
+ 		{
+ 			CoerceToDomain *d = (CoerceToDomain *) expr;
+ 
+ 			if (GetDomainConstraints(d->resulttype) != NIL)
+ 				tab->verify = true;
+ 			expr = (Node *) d->arg;
+ 		}
  		else
! 			tab->rewrite = true;
  	}
  }
  
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