On 19/10/2018 16:00, Michael Paquier wrote:
>> -     if (get_attidentity(RelationGetRelid(rel), attnum))
>> +     if (TupleDescAttr(RelationGetDescr(rel), attnum - 1)->attidentity)
> I find this style heavy, saving Form_pg_attribute into a different
> variable would be more readable in my opinion..

OK, slightly reworked version attached.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From d9a63e636cb5ad41d3105d62dce497141685ac22 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 18 Oct 2018 19:28:28 +0200
Subject: [PATCH v2 1/2] Remove get_atttypmod()

This has been unused since 2004.  get_atttypetypmodcoll() is often a
better alternative.
---
 src/backend/utils/cache/lsyscache.c | 29 +----------------------------
 src/include/utils/lsyscache.h       |  1 -
 2 files changed, 1 insertion(+), 29 deletions(-)

diff --git a/src/backend/utils/cache/lsyscache.c 
b/src/backend/utils/cache/lsyscache.c
index 12b2532d95..c53bda9867 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -880,40 +880,13 @@ get_atttype(Oid relid, AttrNumber attnum)
                return InvalidOid;
 }
 
-/*
- * get_atttypmod
- *
- *             Given the relation id and the attribute number,
- *             return the "atttypmod" field from the attribute relation.
- */
-int32
-get_atttypmod(Oid relid, AttrNumber attnum)
-{
-       HeapTuple       tp;
-
-       tp = SearchSysCache2(ATTNUM,
-                                                ObjectIdGetDatum(relid),
-                                                Int16GetDatum(attnum));
-       if (HeapTupleIsValid(tp))
-       {
-               Form_pg_attribute att_tup = (Form_pg_attribute) GETSTRUCT(tp);
-               int32           result;
-
-               result = att_tup->atttypmod;
-               ReleaseSysCache(tp);
-               return result;
-       }
-       else
-               return -1;
-}
-
 /*
  * get_atttypetypmodcoll
  *
  *             A three-fer: given the relation id and the attribute number,
  *             fetch atttypid, atttypmod, and attcollation in a single cache 
lookup.
  *
- * Unlike the otherwise-similar get_atttype/get_atttypmod, this routine
+ * Unlike the otherwise-similar get_atttype, this routine
  * raises an error if it can't obtain the information.
  */
 void
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 8c57de77c0..23ed5324b5 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -87,7 +87,6 @@ extern char *get_attname(Oid relid, AttrNumber attnum, bool 
missing_ok);
 extern AttrNumber get_attnum(Oid relid, const char *attname);
 extern char get_attidentity(Oid relid, AttrNumber attnum);
 extern Oid     get_atttype(Oid relid, AttrNumber attnum);
-extern int32 get_atttypmod(Oid relid, AttrNumber attnum);
 extern void get_atttypetypmodcoll(Oid relid, AttrNumber attnum,
                                          Oid *typid, int32 *typmod, Oid 
*collid);
 extern char *get_collation_name(Oid colloid);

base-commit: 55853d666ce5d0024c50dc3d223346df28abfa70
-- 
2.19.1

>From 932a53b5012a7df695fc8f26586c17ca697d50f4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 18 Oct 2018 19:27:18 +0200
Subject: [PATCH v2 2/2] Remove get_attidentity()

All existing uses can get this information more easily from the
relation descriptor, so the detour through the syscache is not
necessary.
---
 src/backend/commands/tablecmds.c    | 13 ++++++------
 src/backend/parser/parse_utilcmd.c  |  5 ++++-
 src/backend/utils/cache/lsyscache.c | 32 -----------------------------
 src/include/utils/lsyscache.h       |  1 -
 4 files changed, 11 insertions(+), 40 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e112b4ef4..1a1c80ef8b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5917,6 +5917,7 @@ static ObjectAddress
 ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 {
        HeapTuple       tuple;
+       Form_pg_attribute attTup;
        AttrNumber      attnum;
        Relation        attr_rel;
        List       *indexoidlist;
@@ -5929,13 +5930,12 @@ ATExecDropNotNull(Relation rel, const char *colName, 
LOCKMODE lockmode)
        attr_rel = heap_open(AttributeRelationId, RowExclusiveLock);
 
        tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName);
-
        if (!HeapTupleIsValid(tuple))
                ereport(ERROR,
                                (errcode(ERRCODE_UNDEFINED_COLUMN),
                                 errmsg("column \"%s\" of relation \"%s\" does 
not exist",
                                                colName, 
RelationGetRelationName(rel))));
-
+       attTup = (Form_pg_attribute) GETSTRUCT(tuple);
        attnum = ((Form_pg_attribute) GETSTRUCT(tuple))->attnum;
 
        /* Prevent them from altering a system attribute */
@@ -5945,7 +5945,7 @@ ATExecDropNotNull(Relation rel, const char *colName, 
LOCKMODE lockmode)
                                 errmsg("cannot alter system column \"%s\"",
                                                colName)));
 
-       if (get_attidentity(RelationGetRelid(rel), attnum))
+       if (attTup->attidentity)
                ereport(ERROR,
                                (errcode(ERRCODE_SYNTAX_ERROR),
                                 errmsg("column \"%s\" of relation \"%s\" is an 
identity column",
@@ -6014,9 +6014,9 @@ ATExecDropNotNull(Relation rel, const char *colName, 
LOCKMODE lockmode)
        /*
         * Okay, actually perform the catalog change ... if needed
         */
-       if (((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull)
+       if (attTup->attnotnull)
        {
-               ((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull = false;
+               attTup->attnotnull = false;
 
                CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
 
@@ -6128,6 +6128,7 @@ static ObjectAddress
 ATExecColumnDefault(Relation rel, const char *colName,
                                        Node *newDefault, LOCKMODE lockmode)
 {
+       TupleDesc       tupdesc = RelationGetDescr(rel);
        AttrNumber      attnum;
        ObjectAddress address;
 
@@ -6148,7 +6149,7 @@ ATExecColumnDefault(Relation rel, const char *colName,
                                 errmsg("cannot alter system column \"%s\"",
                                                colName)));
 
-       if (get_attidentity(RelationGetRelid(rel), attnum))
+       if (TupleDescAttr(tupdesc, attnum - 1)->attidentity)
                ereport(ERROR,
                                (errcode(ERRCODE_SYNTAX_ERROR),
                                 errmsg("column \"%s\" of relation \"%s\" is an 
identity column",
diff --git a/src/backend/parser/parse_utilcmd.c 
b/src/backend/parser/parse_utilcmd.c
index d8387d4356..a6a2de94ea 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -2919,6 +2919,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
                                                const char *queryString)
 {
        Relation        rel;
+       TupleDesc       tupdesc;
        ParseState *pstate;
        CreateStmtContext cxt;
        List       *result;
@@ -2938,6 +2939,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 
        /* Caller is responsible for locking the relation */
        rel = relation_open(relid, NoLock);
+       tupdesc = RelationGetDescr(rel);
 
        /* Set up pstate */
        pstate = make_parsestate(NULL);
@@ -3067,7 +3069,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
                                         * if attribute not found, something 
will error about it
                                         * later
                                         */
-                                       if (attnum != InvalidAttrNumber && 
get_attidentity(relid, attnum))
+                                       if (attnum != InvalidAttrNumber &&
+                                               TupleDescAttr(tupdesc, attnum - 
1)->attidentity)
                                        {
                                                Oid                     
seq_relid = getOwnedSequence(relid, attnum);
                                                Oid                     typeOid 
= typenameTypeId(pstate, def->typeName);
diff --git a/src/backend/utils/cache/lsyscache.c 
b/src/backend/utils/cache/lsyscache.c
index c53bda9867..892ddc0d48 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -821,38 +821,6 @@ get_attnum(Oid relid, const char *attname)
                return InvalidAttrNumber;
 }
 
-/*
- * get_attidentity
- *
- *             Given the relation id and the attribute name,
- *             return the "attidentity" field from the attribute relation.
- *
- *             Returns '\0' if not found.
- *
- *             Since no identity is represented by '\0', this can also be used 
as a
- *             Boolean test.
- */
-char
-get_attidentity(Oid relid, AttrNumber attnum)
-{
-       HeapTuple       tp;
-
-       tp = SearchSysCache2(ATTNUM,
-                                                ObjectIdGetDatum(relid),
-                                                Int16GetDatum(attnum));
-       if (HeapTupleIsValid(tp))
-       {
-               Form_pg_attribute att_tup = (Form_pg_attribute) GETSTRUCT(tp);
-               char            result;
-
-               result = att_tup->attidentity;
-               ReleaseSysCache(tp);
-               return result;
-       }
-       else
-               return '\0';
-}
-
 /*
  * get_atttype
  *
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 23ed5324b5..ff1705ad2b 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -85,7 +85,6 @@ extern Oid get_opfamily_proc(Oid opfamily, Oid lefttype, Oid 
righttype,
                                  int16 procnum);
 extern char *get_attname(Oid relid, AttrNumber attnum, bool missing_ok);
 extern AttrNumber get_attnum(Oid relid, const char *attname);
-extern char get_attidentity(Oid relid, AttrNumber attnum);
 extern Oid     get_atttype(Oid relid, AttrNumber attnum);
 extern void get_atttypetypmodcoll(Oid relid, AttrNumber attnum,
                                          Oid *typid, int32 *typmod, Oid 
*collid);
-- 
2.19.1

Reply via email to