about set_attnotnull.

we can make set_attnotnull  look less recursive.
instead of calling find_inheritance_children,
let's just one pass, directly call  find_all_inheritors
overall, I think it would be more intuitive.

please check the attached refactored set_attnotnull.
regress test passed, i only test regress.

I am also beginning to wonder if ATExecSetNotNull inside can also call
find_all_inheritors.
static void
set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum, bool recurse,
			   LOCKMODE lockmode)
{
	HeapTuple	tuple;
	Form_pg_attribute attForm;
	bool		changed = false;

	tuple = SearchSysCacheCopyAttNum(RelationGetRelid(rel), attnum);
	if (!HeapTupleIsValid(tuple))
		elog(ERROR, "cache lookup failed for attribute %d of relation %u",
			 attnum, RelationGetRelid(rel));
	attForm = (Form_pg_attribute) GETSTRUCT(tuple);
	if (!attForm->attnotnull)
	{
		Relation	attr_rel;

		attr_rel = table_open(AttributeRelationId, RowExclusiveLock);

		attForm->attnotnull = true;
		CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);

		table_close(attr_rel, RowExclusiveLock);

		/*
		 * And set up for existing values to be checked, unless another
		 * constraint already proves this.
		 */
		if (wqueue && !NotNullImpliedByRelConstraints(rel, attForm))
		{
			AlteredTableInfo *tab;

			tab = ATGetQueueEntry(wqueue, rel);
			tab->verify_new_notnull = true;
		}

		changed = true;
	}

	if (recurse)
	{
		List	   *child_oids;
		Relation	childrel;
		AttrNumber	childattno;
		HeapTuple	rel_tuple;
		Form_pg_attribute child_attForm;
		const char *attrname;

		attrname = get_attname(RelationGetRelid(rel), attnum, false);
		child_oids = find_all_inheritors(RelationGetRelid(rel), lockmode,
										 NULL);

		if (changed)
			CommandCounterIncrement();

		foreach_oid(childrelid, child_oids)
		{
			/* we alerady dealt with parent rel in above */
			if (childrelid == RelationGetRelid(rel))
				continue;

			childrel = table_open(childrelid, NoLock);
			CheckAlterTableIsSafe(childrel);

			childattno = get_attnum(childrelid, attrname);
			rel_tuple = SearchSysCacheCopyAttNum(childrelid, childattno);

			if (!HeapTupleIsValid(rel_tuple))
				elog(ERROR, "cache lookup failed for attribute %d of relation %s",
					attnum, RelationGetRelationName(childrel));

			child_attForm = (Form_pg_attribute) GETSTRUCT(rel_tuple);
			if (!child_attForm->attnotnull)
			{
				Relation	attr_rel;

				attr_rel = table_open(AttributeRelationId, RowExclusiveLock);

				child_attForm->attnotnull = true;
				CatalogTupleUpdate(attr_rel, &rel_tuple->t_self, rel_tuple);

				table_close(attr_rel, RowExclusiveLock);

				if (wqueue && !NotNullImpliedByRelConstraints(childrel, child_attForm))
				{
					AlteredTableInfo *tab;

					tab = ATGetQueueEntry(wqueue, childrel);
					tab->verify_new_notnull = true;
				}
				changed = true;
			}
			changed = false;
			table_close(childrel, NoLock);
		}
	}
}

Reply via email to