Re: simplify index tuple descriptor initialization

2018-09-12 Thread Peter Eisentraut
On 12/09/2018 15:18, Arthur Zakirov wrote:
> On Mon, Aug 27, 2018 at 04:25:28PM +0200, Peter Eisentraut wrote:
>> Whenever some pg_attribute field is added or changed, a lot of
>> repetitive changes all over the code are necessary.  Here is a small
>> change to remove one such place.
> 
> It looks like a reasonable change to me.
> 
> The code is good and regression tests passed. There is no need to update
> the documentation.
> 
> Marked as Ready for Commiter.

Committed, thanks.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: simplify index tuple descriptor initialization

2018-09-12 Thread Arthur Zakirov
On Mon, Aug 27, 2018 at 04:25:28PM +0200, Peter Eisentraut wrote:
> Whenever some pg_attribute field is added or changed, a lot of
> repetitive changes all over the code are necessary.  Here is a small
> change to remove one such place.

It looks like a reasonable change to me.

The code is good and regression tests passed. There is no need to update
the documentation.

Marked as Ready for Commiter.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company



simplify index tuple descriptor initialization

2018-08-27 Thread Peter Eisentraut
Whenever some pg_attribute field is added or changed, a lot of
repetitive changes all over the code are necessary.  Here is a small
change to remove one such place.

We have two code paths for initializing the tuple descriptor for a new
index: For a normal index, we copy the tuple descriptor from the table
and reset a number of fields that are not applicable to indexes.  For an
expression index, we make a blank tuple descriptor and fill in the
needed fields based on the provided expressions.  As pg_attribute has
grown over time, the number of fields that we need to reset in the first
case is now bigger than the number of fields we actually want to copy,
so it's sensible to do it the other way around: Make a blank descriptor
and copy just the fields we need.  This also allows more code sharing
between the two branches, and it avoids having to touch this code for
almost every unrelated change to the pg_attribute structure.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 7a8d8bd13fd1cdaf18de21f4d45df4269aa71d10 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Mon, 27 Aug 2018 15:50:50 +0200
Subject: [PATCH] Simplify index tuple descriptor initialization

We have two code paths for initializing the tuple descriptor for a new
index: For a normal index, we copy the tuple descriptor from the table
and reset a number of fields that are not applicable to indexes.  For an
expression index, we make a blank tuple descriptor and fill in the
needed fields based on the provided expressions.  As pg_attribute has
grown over time, the number of fields that we need to reset in the first
case is now bigger than the number of fields we actually want to copy,
so it's sensible to do it the other way around: Make a blank descriptor
and copy just the fields we need.  This also allows more code sharing
between the two branches, and it avoids having to touch this code for
almost every unrelated change to the pg_attribute structure.
---
 src/backend/catalog/index.c | 57 +++--
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b256054908..636fe9c740 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -319,9 +319,7 @@ ConstructTupleDescriptor(Relation heapRelation,
indexTupDesc = CreateTemplateTupleDesc(numatts, false);
 
/*
-* For simple index columns, we copy the pg_attribute row from the 
parent
-* relation and modify it as necessary.  For expressions we have to cons
-* up a pg_attribute row the hard way.
+* Fill in the pg_attribute row.
 */
for (i = 0; i < numatts; i++)
{
@@ -332,6 +330,19 @@ ConstructTupleDescriptor(Relation heapRelation,
Form_pg_opclass opclassTup;
Oid keyType;
 
+   MemSet(to, 0, ATTRIBUTE_FIXED_PART_SIZE);
+   to->attnum = i + 1;
+   to->attstattarget = -1;
+   to->attcacheoff = -1;
+   to->attislocal = true;
+   to->attcollation = (i < numkeyatts) ?
+   collationObjectId[i] : InvalidOid;
+
+   /*
+* For simple index columns, we copy some pg_attribute fields 
from the
+* parent relation.  For expressions we have to look at the 
expression
+* result.
+*/
if (atnum != 0)
{
/* Simple index column */
@@ -356,36 +367,20 @@ ConstructTupleDescriptor(Relation heapRelation,
 
AttrNumberGetAttrOffset(atnum));
}
 
-   /*
-* now that we've determined the "from", let's copy the 
tuple desc
-* data...
-*/
-   memcpy(to, from, ATTRIBUTE_FIXED_PART_SIZE);
-
-   /*
-* Fix the stuff that should not be the same as the 
underlying
-* attr
-*/
-   to->attnum = i + 1;
-
-   to->attstattarget = -1;
-   to->attcacheoff = -1;
-   to->attnotnull = false;
-   to->atthasdef = false;
-   to->atthasmissing = false;
-   to->attidentity = '\0';
-   to->attislocal = true;
-   to->attinhcount = 0;
-   to->attcollation = (i < numkeyatts) ?
-   collationObjectId[i] : InvalidOid;
+   namecpy(&to->attname, &from->at