On 2013-12-20 13:22:07 +0100, Andres Freund wrote:
> On 2013-12-20 07:12:01 -0500, Robert Haas wrote:
> > I think the root of the problem is that nobody's very eager to add
> > more hidden system catalog columns because each one bloats
> > pg_attribute significantly.
> 
> I think that part is actually solveable - there's no real need for them
> to be real columns, present in pg_attribute, things could easily enough be
> setup during parse analysis.

I've spent some time yesterday hacking up a prototype for this. The
amount of effort seems reasonable, although the attached patch certainly
doesn't do all the neccessary things. The regression tests pass.

In the prototype only oid keeps it's pg_attribute entry, everything else
uses the static entries via
SystemAttributeDefinition()/SystemAttributeByName(). We might want to
remove oids as well, but it seems reasonable to continue allowing
defining column permissions on it. And it's likely the attribute that's
most likely ingrained deeply in user applications.

> The bigger problem I see is that adding
> more useful columns will cause name clashes, which will probably
> prohibit improvements in that direction.

I wonder if we could solve that by naming new columns like
"system:attribute" or similar. Not pretty, but maybe better than the
alternatives.

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 3a5692575c0077601cfba938239f4dd6f74de3c5 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Mon, 23 Dec 2013 13:44:36 +0100
Subject: [PATCH] prototype-patch: remove system columns from pg_attribute

---
 src/backend/access/common/heaptuple.c |  14 ++++
 src/backend/catalog/aclchk.c          |  27 +++---
 src/backend/catalog/heap.c            |  18 ++--
 src/backend/commands/tablecmds.c      | 154 +++++++++++++++++++---------------
 src/backend/parser/parse_relation.c   |  65 ++++++++++----
 src/backend/utils/cache/lsyscache.c   |  11 ++-
 src/include/access/sysattr.h          |   3 +-
 7 files changed, 188 insertions(+), 104 deletions(-)

diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 347d616..99471ab 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -59,7 +59,9 @@
 
 #include "access/sysattr.h"
 #include "access/tuptoaster.h"
+#include "access/transam.h"
 #include "executor/tuptable.h"
+#include "storage/procarray.h"
 
 
 /* Does att's datatype allow packing into the 1-byte-header varlena format? */
@@ -286,6 +288,7 @@ heap_attisnull(HeapTuple tup, int attnum)
 		case MinCommandIdAttributeNumber:
 		case MaxTransactionIdAttributeNumber:
 		case MaxCommandIdAttributeNumber:
+		case CookedMaxTransactionIdAttributeNumber:
 			/* these are never null */
 			break;
 
@@ -558,6 +561,17 @@ heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull)
 		case TableOidAttributeNumber:
 			result = ObjectIdGetDatum(tup->t_tableOid);
 			break;
+		case CookedMaxTransactionIdAttributeNumber:
+			{
+				TransactionId xid;
+				xid = HeapTupleHeaderGetUpdateXid(tup->t_data);
+				if (TransactionIdIsValid(xid) &&
+					!TransactionIdIsInProgress(xid) &&
+					!TransactionIdDidCommit(xid))
+					xid = InvalidTransactionId;
+				result = TransactionIdGetDatum(xid);
+			}
+			break;
 		default:
 			elog(ERROR, "invalid attnum: %d", attnum);
 			result = 0;			/* keep compiler quiet */
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 5a46fd9..807e274 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -1532,16 +1532,19 @@ expand_all_col_privileges(Oid table_oid, Form_pg_class classForm,
 		if (classForm->relkind == RELKIND_VIEW && curr_att < 0)
 			continue;
 
-		attTuple = SearchSysCache2(ATTNUM,
-								   ObjectIdGetDatum(table_oid),
-								   Int16GetDatum(curr_att));
-		if (!HeapTupleIsValid(attTuple))
-			elog(ERROR, "cache lookup failed for attribute %d of relation %u",
-				 curr_att, table_oid);
-
-		isdropped = ((Form_pg_attribute) GETSTRUCT(attTuple))->attisdropped;
-
-		ReleaseSysCache(attTuple);
+		if (curr_att < 0)
+			isdropped = false;
+		else
+		{
+			attTuple = SearchSysCache2(ATTNUM,
+									   ObjectIdGetDatum(table_oid),
+									   Int16GetDatum(curr_att));
+			if (!HeapTupleIsValid(attTuple))
+				elog(ERROR, "cache lookup failed for attribute %d of relation %u",
+					 curr_att, table_oid);
+			isdropped = ((Form_pg_attribute) GETSTRUCT(attTuple))->attisdropped;
+			ReleaseSysCache(attTuple);
+		}
 
 		/* ignore dropped columns */
 		if (isdropped)
@@ -1579,6 +1582,10 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid, const char *relname,
 	Oid		   *oldmembers;
 	Oid		   *newmembers;
 
+	/* FIXME: don't set column permissions on system columns */
+	if (attnum < 0)
+		return;
+
 	attr_tuple = SearchSysCache2(ATTNUM,
 								 ObjectIdGetDatum(relOid),
 								 Int16GetDatum(attnum));
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 032a20e..476bb34 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -180,7 +180,14 @@ static FormData_pg_attribute a7 = {
 	true, 'p', 'i', true, false, false, true, 0
 };
 
-static const Form_pg_attribute SysAtt[] = {&a1, &a2, &a3, &a4, &a5, &a6, &a7};
+static FormData_pg_attribute a8 = {
+	0, {"cooked_xmax"}, XIDOID, 0, sizeof(TransactionId),
+	CookedMaxTransactionIdAttributeNumber, 0, -1, -1,
+	true, 'p', 'i', true, false, false, true, 0
+};
+
+
+static const Form_pg_attribute SysAtt[] = {&a1, &a2, &a3, &a4, &a5, &a6, &a7, &a8};
 
 /*
  * This function returns a Form_pg_attribute pointer for a system attribute.
@@ -709,9 +716,8 @@ AddNewAttributeTuples(Oid new_rel_oid,
 	}
 
 	/*
-	 * Next we add the system attributes.  Skip OID if rel has no OIDs. Skip
-	 * all for a view or type relation.  We don't bother with making datatype
-	 * dependencies here, since presumably all these types are pinned.
+	 * Next we add the system attributes we want to add as actual columns,
+	 * currently that's only the oid.
 	 */
 	if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
 	{
@@ -720,8 +726,8 @@ AddNewAttributeTuples(Oid new_rel_oid,
 			FormData_pg_attribute attStruct;
 
 			/* skip OID where appropriate */
-			if (!tupdesc->tdhasoid &&
-				SysAtt[i]->attnum == ObjectIdAttributeNumber)
+			if (!tupdesc->tdhasoid ||
+				SysAtt[i]->attnum != ObjectIdAttributeNumber)
 				continue;
 
 			memcpy(&attStruct, (char *) SysAtt[i], sizeof(FormData_pg_attribute));
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b9cd88d..2003171 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4738,7 +4738,17 @@ static void
 check_for_column_name_collision(Relation rel, const char *colname)
 {
 	HeapTuple	attTuple;
-	int			attnum;
+
+	/*
+	 * We throw a different error message for conflicts with system column
+	 * names, since they are normally not shown and the user might otherwise
+	 * be confused about the reason for the conflict.
+	 */
+	if (SystemAttributeByName(colname, rel->rd_rel->relhasoids) != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_DUPLICATE_COLUMN),
+				 errmsg("column name \"%s\" conflicts with a system column name",
+						colname)));
 
 	/*
 	 * this test is deliberately not attisdropped-aware, since if one tries to
@@ -4750,24 +4760,12 @@ check_for_column_name_collision(Relation rel, const char *colname)
 	if (!HeapTupleIsValid(attTuple))
 		return;
 
-	attnum = ((Form_pg_attribute) GETSTRUCT(attTuple))->attnum;
 	ReleaseSysCache(attTuple);
 
-	/*
-	 * We throw a different error message for conflicts with system column
-	 * names, since they are normally not shown and the user might otherwise
-	 * be confused about the reason for the conflict.
-	 */
-	if (attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_DUPLICATE_COLUMN),
-			 errmsg("column name \"%s\" conflicts with a system column name",
-					colname)));
-	else
-		ereport(ERROR,
-				(errcode(ERRCODE_DUPLICATE_COLUMN),
-				 errmsg("column \"%s\" of relation \"%s\" already exists",
-						colname, RelationGetRelationName(rel))));
+	ereport(ERROR,
+			(errcode(ERRCODE_DUPLICATE_COLUMN),
+			 errmsg("column \"%s\" of relation \"%s\" already exists",
+					colname, RelationGetRelationName(rel))));
 }
 
 /*
@@ -4856,6 +4854,13 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 	 */
 	attr_rel = heap_open(AttributeRelationId, RowExclusiveLock);
 
+	/* Prevent them from altering a system attribute */
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
 	tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName);
 
 	if (!HeapTupleIsValid(tuple))
@@ -4866,12 +4871,7 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 
 	attnum = ((Form_pg_attribute) GETSTRUCT(tuple))->attnum;
 
-	/* Prevent them from altering a system attribute */
-	if (attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"",
-						colName)));
+	Assert(attnum > 0);
 
 	/*
 	 * Check that the attribute is not in a primary key
@@ -4951,6 +4951,13 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
 	 */
 	attr_rel = heap_open(AttributeRelationId, RowExclusiveLock);
 
+	/* Prevent them from altering a system attribute */
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids) != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
 	tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName);
 
 	if (!HeapTupleIsValid(tuple))
@@ -4961,12 +4968,7 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
 
 	attnum = ((Form_pg_attribute) GETSTRUCT(tuple))->attnum;
 
-	/* Prevent them from altering a system attribute */
-	if (attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"",
-						colName)));
+	Assert(attnum > 0);
 
 	/*
 	 * Okay, actually perform the catalog change ... if needed
@@ -4999,6 +5001,13 @@ ATExecColumnDefault(Relation rel, const char *colName,
 {
 	AttrNumber	attnum;
 
+	/* Prevent them from altering a system attribute */
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
 	/*
 	 * get the number of the attribute
 	 */
@@ -5009,12 +5018,7 @@ ATExecColumnDefault(Relation rel, const char *colName,
 				 errmsg("column \"%s\" of relation \"%s\" does not exist",
 						colName, RelationGetRelationName(rel))));
 
-	/* Prevent them from altering a system attribute */
-	if (attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"",
-						colName)));
+	Assert(attnum > 0);
 
 	/*
 	 * Remove any old default for the column.  We use RESTRICT here for
@@ -5105,6 +5109,13 @@ ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE
 
 	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
 
+	/* Prevent them from altering a system attribute */
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids) != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
 	tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName);
 
 	if (!HeapTupleIsValid(tuple))
@@ -5114,11 +5125,7 @@ ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE
 						colName, RelationGetRelationName(rel))));
 	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
 
-	if (attrtuple->attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"",
-						colName)));
+	Assert(attrtuple->attnum > 0);
 
 	attrtuple->attstattarget = newtarget;
 
@@ -5152,6 +5159,13 @@ ATExecSetOptions(Relation rel, const char *colName, Node *options,
 
 	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
 
+	/* Prevent them from altering a system attribute */
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids) != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
 	tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
 
 	if (!HeapTupleIsValid(tuple))
@@ -5161,12 +5175,6 @@ ATExecSetOptions(Relation rel, const char *colName, Node *options,
 						colName, RelationGetRelationName(rel))));
 	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
 
-	if (attrtuple->attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"",
-						colName)));
-
 	/* Generate new proposed attoptions (text array) */
 	Assert(IsA(options, List));
 	datum = SysCacheGetAttr(ATTNAME, tuple, Anum_pg_attribute_attoptions,
@@ -5236,6 +5244,13 @@ ATExecSetStorage(Relation rel, const char *colName, Node *newValue, LOCKMODE loc
 
 	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
 
+	/* Prevent them from altering a system attribute */
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids) != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
 	tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName);
 
 	if (!HeapTupleIsValid(tuple))
@@ -5245,11 +5260,7 @@ ATExecSetStorage(Relation rel, const char *colName, Node *newValue, LOCKMODE loc
 						colName, RelationGetRelationName(rel))));
 	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
 
-	if (attrtuple->attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"",
-						colName)));
+	Assert(attrtuple->attnum > 0);
 
 	/*
 	 * safety check: do not allow toasted storage modes unless column datatype
@@ -5319,6 +5330,14 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
 	if (recursing)
 		ATSimplePermissions(rel, ATT_TABLE);
 
+	/* Can't drop a system attribute, except OID */
+	targetatt = SystemAttributeByName(colName, rel->rd_rel->relhasoids);
+	if (targetatt != NULL && targetatt->attnum != ObjectIdAttributeNumber)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot drop system column \"%s\"",
+						colName)));
+
 	/*
 	 * get the number of the attribute
 	 */
@@ -5344,13 +5363,6 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
 
 	attnum = targetatt->attnum;
 
-	/* Can't drop a system attribute, except OID */
-	if (attnum <= 0 && attnum != ObjectIdAttributeNumber)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot drop system column \"%s\"",
-						colName)));
-
 	/* Don't drop inherited columns */
 	if (targetatt->attinhcount > 0 && !recursing)
 		ereport(ERROR,
@@ -7399,6 +7411,13 @@ ATPrepAlterColumnType(List **wqueue,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot alter column type of typed table")));
 
+	/* Can't alter a system attribute */
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids) != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
 	/* lookup the attribute so we can check inheritance status */
 	tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
 	if (!HeapTupleIsValid(tuple))
@@ -7409,12 +7428,7 @@ ATPrepAlterColumnType(List **wqueue,
 	attTup = (Form_pg_attribute) GETSTRUCT(tuple);
 	attnum = attTup->attnum;
 
-	/* Can't alter a system attribute */
-	if (attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"",
-						colName)));
+	Assert(attnum > 0);
 
 	/* Don't alter inherited columns */
 	if (attTup->attinhcount > 0 && !recursing)
@@ -7996,6 +8010,12 @@ ATExecAlterColumnGenericOptions(Relation rel,
 	ReleaseSysCache(tuple);
 
 	attrel = heap_open(AttributeRelationId, RowExclusiveLock);
+
+	if (SystemAttributeByName(colName, rel->rd_rel->relhasoids) != NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"", colName)));
+
 	tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
 	if (!HeapTupleIsValid(tuple))
 		ereport(ERROR,
@@ -8003,13 +8023,9 @@ ATExecAlterColumnGenericOptions(Relation rel,
 				 errmsg("column \"%s\" of relation \"%s\" does not exist",
 						colName, RelationGetRelationName(rel))));
 
-	/* Prevent them from altering a system attribute */
 	atttableform = (Form_pg_attribute) GETSTRUCT(tuple);
-	if (atttableform->attnum <= 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot alter system column \"%s\"", colName)));
 
+	Assert(atttableform->attnum > 0);
 
 	/* Initialize buffers for new tuple values */
 	memset(repl_val, 0, sizeof(repl_val));
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index cd8d75e..c8dcd47 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -573,18 +573,29 @@ scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte, char *colname,
 						colname),
 					 parser_errposition(pstate, location)));
 
-		if (attnum != InvalidAttrNumber)
+		if (attnum != InvalidAttrNumber &&
+			(rte->relkind != RELKIND_VIEW && rte->relkind != RELKIND_COMPOSITE_TYPE))
 		{
-			/* now check to see if column actually is defined */
-			if (SearchSysCacheExists2(ATTNUM,
-									  ObjectIdGetDatum(rte->relid),
-									  Int16GetDatum(attnum)))
+			/* verify that relation has oids */
+			if (attnum == ObjectIdAttributeNumber)
 			{
-				var = make_var(pstate, rte, attnum, location);
-				/* Require read access to the column */
-				markVarForSelectPriv(pstate, var, rte);
-				result = (Node *) var;
+				HeapTuple tp;
+				Form_pg_class class_tup;
+				bool		hasoids;
+				tp = SearchSysCache1(RELOID, ObjectIdGetDatum(rte->relid));
+				if (!HeapTupleIsValid(tp))
+					elog(ERROR, "cache lookup failed for relation %u",
+						 rte->relid);
+				class_tup = (Form_pg_class) GETSTRUCT(tp);
+				hasoids = class_tup->relhasoids;
+				ReleaseSysCache(tp);
+
+				if (!hasoids)
+					return result;
 			}
+			var = make_var(pstate, rte, attnum, location);
+			/* XXX: Ignoring column permissions here! */
+			result = (Node *) var;
 		}
 	}
 
@@ -2323,14 +2334,33 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
 				/* Plain relation RTE --- get the attribute's type info */
 				HeapTuple	tp;
 				Form_pg_attribute att_tup;
+				Form_pg_class class_tup;
+				bool syscol = attnum < 0;
 
-				tp = SearchSysCache2(ATTNUM,
-									 ObjectIdGetDatum(rte->relid),
-									 Int16GetDatum(attnum));
-				if (!HeapTupleIsValid(tp))		/* shouldn't happen */
-					elog(ERROR, "cache lookup failed for attribute %d of relation %u",
-						 attnum, rte->relid);
-				att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+				if (syscol)
+				{
+					tp = SearchSysCache1(RELOID, ObjectIdGetDatum(rte->relid));
+					if (!HeapTupleIsValid(tp))
+						elog(ERROR, "cache lookup failed for relation %u",
+							 rte->relid);
+					class_tup = (Form_pg_class) GETSTRUCT(tp);
+					if (attnum == ObjectIdAttributeNumber && !class_tup->relhasoids)
+						elog(ERROR, "syscol access to %s in %d", NameStr(class_tup->relname), attnum);
+
+					att_tup = SystemAttributeDefinition(attnum, class_tup->relhasoids);
+					ReleaseSysCache(tp);
+				}
+				else
+				{
+					tp = SearchSysCache2(ATTNUM,
+										 ObjectIdGetDatum(rte->relid),
+										 Int16GetDatum(attnum));
+
+					if (!HeapTupleIsValid(tp))		/* shouldn't happen */
+						elog(ERROR, "cache lookup failed for attribute %d of relation %u",
+							 attnum, rte->relid);
+					att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+				}
 
 				/*
 				 * If dropped column, pretend it ain't there.  See notes in
@@ -2345,7 +2375,8 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
 				*vartype = att_tup->atttypid;
 				*vartypmod = att_tup->atttypmod;
 				*varcollid = att_tup->attcollation;
-				ReleaseSysCache(tp);
+				if (!syscol)
+					ReleaseSysCache(tp);
 			}
 			break;
 		case RTE_SUBQUERY:
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 5865962..49e8877 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -19,6 +19,7 @@
 #include "access/htup_details.h"
 #include "access/nbtree.h"
 #include "bootstrap/bootstrap.h"
+#include "catalog/heap.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_collation.h"
@@ -826,15 +827,23 @@ char *
 get_attname(Oid relid, AttrNumber attnum)
 {
 	HeapTuple	tp;
+	Form_pg_attribute att_tup;
+
+	if (attnum < 0)
+	{
+		att_tup = SystemAttributeDefinition(attnum, true);
+		return pstrdup(NameStr(att_tup->attname));
+	}
 
 	tp = SearchSysCache2(ATTNUM,
 						 ObjectIdGetDatum(relid),
 						 Int16GetDatum(attnum));
 	if (HeapTupleIsValid(tp))
 	{
-		Form_pg_attribute att_tup = (Form_pg_attribute) GETSTRUCT(tp);
 		char	   *result;
 
+		att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+
 		result = pstrdup(NameStr(att_tup->attname));
 		ReleaseSysCache(tp);
 		return result;
diff --git a/src/include/access/sysattr.h b/src/include/access/sysattr.h
index f9b2482..0865d8f 100644
--- a/src/include/access/sysattr.h
+++ b/src/include/access/sysattr.h
@@ -25,6 +25,7 @@
 #define MaxTransactionIdAttributeNumber			(-5)
 #define MaxCommandIdAttributeNumber				(-6)
 #define TableOidAttributeNumber					(-7)
-#define FirstLowInvalidHeapAttributeNumber		(-8)
+#define CookedMaxTransactionIdAttributeNumber	(-8)
+#define FirstLowInvalidHeapAttributeNumber		(-9)
 
 #endif   /* SYSATTR_H */
-- 
1.8.3.251.g1462b67

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to