[PATCH] elimination of code duplication in DefineOpFamily()

Started by KaiGai Koheialmost 16 years ago3 messages
#1KaiGai Kohei
kaigai@ak.jp.nec.com
1 attachment(s)

It looks like for me the bottom half of the DefineOpFamily() is
a block copied and pasted from CreateOpFamily().
It has identical logic, variable names and source code comments.

Perhaps, we can replace this code block by CreateOpFamily() call.

Thanks,
--
KaiGai Kohei <kaigai@ak.jp.nec.com>

Attachments:

pgsql-cleanup-defineopfamily.patchapplication/octect-stream; name=pgsql-cleanup-defineopfamily.patchDownload
*** a/src/backend/commands/opclasscmds.c
--- b/src/backend/commands/opclasscmds.c
***************
*** 640,655 **** DefineOpFamily(CreateOpFamilyStmt *stmt)
  {
  	char	   *opfname;		/* name of opfamily we're creating */
  	Oid			amoid,			/* our AM's oid */
! 				namespaceoid,	/* namespace to create opfamily in */
! 				opfamilyoid;	/* oid of opfamily we create */
! 	Relation	rel;
  	HeapTuple	tup;
- 	Datum		values[Natts_pg_opfamily];
- 	bool		nulls[Natts_pg_opfamily];
  	AclResult	aclresult;
- 	NameData	opfName;
- 	ObjectAddress myself,
- 				referenced;
  
  	/* Convert list of names to a name and namespace */
  	namespaceoid = QualifiedNameGetCreationNamespace(stmt->opfamilyname,
--- 640,648 ----
  {
  	char	   *opfname;		/* name of opfamily we're creating */
  	Oid			amoid,			/* our AM's oid */
! 				namespaceoid;	/* namespace to create opfamily in */
  	HeapTuple	tup;
  	AclResult	aclresult;
  
  	/* Convert list of names to a name and namespace */
  	namespaceoid = QualifiedNameGetCreationNamespace(stmt->opfamilyname,
***************
*** 686,745 **** DefineOpFamily(CreateOpFamilyStmt *stmt)
  				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
  				 errmsg("must be superuser to create an operator family")));
  
! 	rel = heap_open(OperatorFamilyRelationId, RowExclusiveLock);
! 
! 	/*
! 	 * Make sure there is no existing opfamily of this name (this is just to
! 	 * give a more friendly error message than "duplicate key").
! 	 */
! 	if (SearchSysCacheExists3(OPFAMILYAMNAMENSP,
! 							  ObjectIdGetDatum(amoid),
! 							  CStringGetDatum(opfname),
! 							  ObjectIdGetDatum(namespaceoid)))
! 		ereport(ERROR,
! 				(errcode(ERRCODE_DUPLICATE_OBJECT),
! 				 errmsg("operator family \"%s\" for access method \"%s\" already exists",
! 						opfname, stmt->amname)));
! 
! 	/*
! 	 * Okay, let's create the pg_opfamily entry.
! 	 */
! 	memset(values, 0, sizeof(values));
! 	memset(nulls, false, sizeof(nulls));
! 
! 	values[Anum_pg_opfamily_opfmethod - 1] = ObjectIdGetDatum(amoid);
! 	namestrcpy(&opfName, opfname);
! 	values[Anum_pg_opfamily_opfname - 1] = NameGetDatum(&opfName);
! 	values[Anum_pg_opfamily_opfnamespace - 1] = ObjectIdGetDatum(namespaceoid);
! 	values[Anum_pg_opfamily_opfowner - 1] = ObjectIdGetDatum(GetUserId());
! 
! 	tup = heap_form_tuple(rel->rd_att, values, nulls);
! 
! 	opfamilyoid = simple_heap_insert(rel, tup);
! 
! 	CatalogUpdateIndexes(rel, tup);
! 
! 	heap_freetuple(tup);
! 
! 	/*
! 	 * Create dependencies for the opfamily proper.  Note: we do not create a
! 	 * dependency link to the AM, because we don't currently support DROP
! 	 * ACCESS METHOD.
! 	 */
! 	myself.classId = OperatorFamilyRelationId;
! 	myself.objectId = opfamilyoid;
! 	myself.objectSubId = 0;
! 
! 	/* dependency on namespace */
! 	referenced.classId = NamespaceRelationId;
! 	referenced.objectId = namespaceoid;
! 	referenced.objectSubId = 0;
! 	recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
! 
! 	/* dependency on owner */
! 	recordDependencyOnOwner(OperatorFamilyRelationId, opfamilyoid, GetUserId());
! 
! 	heap_close(rel, RowExclusiveLock);
  }
  
  
--- 679,686 ----
  				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
  				 errmsg("must be superuser to create an operator family")));
  
! 	/* Update pg_opfamily catalog */
! 	CreateOpFamily(stmt->amname, opfname, namespaceoid, amoid);
  }
  
  
#2Brent Dombrowski
brent.dombrowski@gmail.com
In reply to: KaiGai Kohei (#1)
Re: [PATCH] elimination of code duplication in DefineOpFamily()

I have not been able to find any comments or discussion on this patch.

Contents and Purpose:
================
This patch removes duplicate code in opclasscmds.c. It removes the
duplicate code from DefineOpFamily by calling CreateOpFamily.

No new regression test or documentation are included with the patch.
Since it is not adding, removing, or changing any functionality, they
are most likely not necessary. The function call parameters remain the same.

Initial Run
========
The patch applied cleanly to HEAD. The regression tests pass both before
and after the patch.

Performance
=========
No changes in performance were observed. I do not have an appropriate
setup to properly test for performance impacts. Since this changes a
define function, I would not expect it to be frequently used.

Conclusion
========
The patch eliminates duplicate code as expected.

Brent Dombrowski

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Brent Dombrowski (#2)
Re: [PATCH] elimination of code duplication in DefineOpFamily()

Brent Dombrowski <brent.dombrowski@gmail.com> writes:

[ review of KaiGai-san's patch ]

Committed. Thanks for the patch, and the review.

regards, tom lane