Skip to content

Commit

Permalink
Re-order pg_attribute columns to eliminate some padding space.
Browse files Browse the repository at this point in the history 
Now that attcompression is just a char, there's a lot of wasted
padding space after it.  Move it into the group of char-wide
columns to save a net of 4 bytes per pg_attribute entry.  While
we're at it, swap the order of attstorage and attalign to make for
a more logical grouping of these columns.

Also re-order actions in related code to match the new field ordering.

This patch also fixes one outright bug: equalTupleDescs() failed to
compare attcompression.  That could, for example, cause relcache
reload to fail to adopt a new value following a change.

Michael Paquier and Tom Lane, per a gripe from Andres Freund.

Discussion: https://postgr.es/m/20210517204803.iyk5wwvwgtjcmc5w@alap3.anarazel.de
  • Loading branch information
@tglsfdc
tglsfdc committed May 23, 2021
1 parent bc2a389 commit f5024d8
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 73 deletions.
28 changes: 14 additions & 14 deletions doc/src/sgml/catalogs.sgml
View file
Expand Up @@ -1236,6 +1236,15 @@
</para></entry>
</row>

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>attalign</structfield> <type>char</type>
</para>
<para>
A copy of <literal>pg_type.typalign</literal> of this column's type
</para></entry>
</row>

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>attstorage</structfield> <type>char</type>
Expand All @@ -1249,10 +1258,13 @@

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>attalign</structfield> <type>char</type>
<structfield>attcompression</structfield> <type>char</type>
</para>
<para>
A copy of <literal>pg_type.typalign</literal> of this column's type
The current compression method of the column. If it is an invalid
compression method (<literal>'\0'</literal>) then column data will not
be compressed. Otherwise, <literal>'p'</literal> = pglz compression or
<literal>'l'</literal> = <productname>LZ4</productname> compression.
</para></entry>
</row>

Expand Down Expand Up @@ -1355,18 +1367,6 @@
</para></entry>
</row>

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>attcompression</structfield> <type>char</type>
</para>
<para>
The current compression method of the column. If it is an invalid
compression method (<literal>'\0'</literal>) then column data will not
be compressed. Otherwise, <literal>'p'</literal> = pglz compression or
<literal>'l'</literal> = <productname>LZ4</productname> compression.
</para></entry>
</row>

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>attacl</structfield> <type>aclitem[]</type>
Expand Down
16 changes: 11 additions & 5 deletions src/backend/access/common/tupdesc.c
View file
Expand Up @@ -424,7 +424,8 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
* general it seems safer to check them always.
*
* attcacheoff must NOT be checked since it's possibly not set in both
* copies.
* copies. We also intentionally ignore atthasmissing, since that's
* not very relevant in tupdescs, which lack the attmissingval field.
*/
if (strcmp(NameStr(attr1->attname), NameStr(attr2->attname)) != 0)
return false;
Expand All @@ -440,9 +441,11 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
return false;
if (attr1->attbyval != attr2->attbyval)
return false;
if (attr1->attalign != attr2->attalign)
return false;
if (attr1->attstorage != attr2->attstorage)
return false;
if (attr1->attalign != attr2->attalign)
if (attr1->attcompression != attr2->attcompression)
return false;
if (attr1->attnotnull != attr2->attnotnull)
return false;
Expand All @@ -460,7 +463,7 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
return false;
if (attr1->attcollation != attr2->attcollation)
return false;
/* attacl, attoptions and attfdwoptions are not even present... */
/* variable-length fields are not even present... */
}

if (tupdesc1->constr != NULL)
Expand Down Expand Up @@ -639,12 +642,11 @@ TupleDescInitEntry(TupleDesc desc,
att->attbyval = typeForm->typbyval;
att->attalign = typeForm->typalign;
att->attstorage = typeForm->typstorage;
att->attcollation = typeForm->typcollation;

if (IsStorageCompressible(typeForm->typstorage))
att->attcompression = GetDefaultToastCompression();
else
att->attcompression = InvalidCompressionMethod;
att->attcollation = typeForm->typcollation;

ReleaseSysCache(tuple);
}
Expand Down Expand Up @@ -709,6 +711,7 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
att->attbyval = false;
att->attalign = TYPALIGN_INT;
att->attstorage = TYPSTORAGE_EXTENDED;
att->attcompression = GetDefaultToastCompression();
att->attcollation = DEFAULT_COLLATION_OID;
break;

Expand All @@ -717,6 +720,7 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
att->attbyval = true;
att->attalign = TYPALIGN_CHAR;
att->attstorage = TYPSTORAGE_PLAIN;
att->attcompression = InvalidCompressionMethod;
att->attcollation = InvalidOid;
break;

Expand All @@ -725,6 +729,7 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
att->attbyval = true;
att->attalign = TYPALIGN_INT;
att->attstorage = TYPSTORAGE_PLAIN;
att->attcompression = InvalidCompressionMethod;
att->attcollation = InvalidOid;
break;

Expand All @@ -733,6 +738,7 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
att->attbyval = FLOAT8PASSBYVAL;
att->attalign = TYPALIGN_DOUBLE;
att->attstorage = TYPSTORAGE_PLAIN;
att->attcompression = InvalidCompressionMethod;
att->attcollation = InvalidOid;
break;

Expand Down
4 changes: 2 additions & 2 deletions src/backend/access/spgist/spgutils.c
View file
Expand Up @@ -165,8 +165,8 @@ fillTypeDesc(SpGistTypeDesc *desc, Oid type)
typtup = (Form_pg_type) GETSTRUCT(tp);
desc->attlen = typtup->typlen;
desc->attbyval = typtup->typbyval;
desc->attstorage = typtup->typstorage;
desc->attalign = typtup->typalign;
desc->attstorage = typtup->typstorage;
ReleaseSysCache(tp);
}

Expand Down Expand Up @@ -304,8 +304,8 @@ getSpGistTupleDesc(Relation index, SpGistTypeDesc *keyType)
att->attalign = keyType->attalign;
att->attstorage = keyType->attstorage;
/* We shouldn't need to bother with making these valid: */
att->attcollation = InvalidOid;
att->attcompression = InvalidCompressionMethod;
att->attcollation = InvalidOid;
/* In case we changed typlen, we'd better reset following offsets */
for (int i = spgFirstIncludeColumn; i < outTupDesc->natts; i++)
TupleDescAttr(outTupDesc, i)->attcacheoff = -1;
Expand Down
13 changes: 7 additions & 6 deletions src/backend/bootstrap/bootstrap.c
View file
Expand Up @@ -699,8 +699,8 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
attrtypes[attnum]->atttypid = Ap->am_oid;
attrtypes[attnum]->attlen = Ap->am_typ.typlen;
attrtypes[attnum]->attbyval = Ap->am_typ.typbyval;
attrtypes[attnum]->attstorage = Ap->am_typ.typstorage;
attrtypes[attnum]->attalign = Ap->am_typ.typalign;
attrtypes[attnum]->attstorage = Ap->am_typ.typstorage;
attrtypes[attnum]->attcollation = Ap->am_typ.typcollation;
/* if an array type, assume 1-dimensional attribute */
if (Ap->am_typ.typelem != InvalidOid && Ap->am_typ.typlen < 0)
Expand All @@ -713,8 +713,8 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
attrtypes[attnum]->atttypid = TypInfo[typeoid].oid;
attrtypes[attnum]->attlen = TypInfo[typeoid].len;
attrtypes[attnum]->attbyval = TypInfo[typeoid].byval;
attrtypes[attnum]->attstorage = TypInfo[typeoid].storage;
attrtypes[attnum]->attalign = TypInfo[typeoid].align;
attrtypes[attnum]->attstorage = TypInfo[typeoid].storage;
attrtypes[attnum]->attcollation = TypInfo[typeoid].collation;
/* if an array type, assume 1-dimensional attribute */
if (TypInfo[typeoid].elem != InvalidOid &&
Expand All @@ -724,6 +724,11 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
attrtypes[attnum]->attndims = 0;
}

if (IsStorageCompressible(attrtypes[attnum]->attstorage))
attrtypes[attnum]->attcompression = GetDefaultToastCompression();
else
attrtypes[attnum]->attcompression = InvalidCompressionMethod;

/*
* If a system catalog column is collation-aware, force it to use C
* collation, so that its behavior is independent of the database's
Expand All @@ -737,10 +742,6 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
attrtypes[attnum]->attcacheoff = -1;
attrtypes[attnum]->atttypmod = -1;
attrtypes[attnum]->attislocal = true;
if (IsStorageCompressible(attrtypes[attnum]->attstorage))
attrtypes[attnum]->attcompression = GetDefaultToastCompression();
else
attrtypes[attnum]->attcompression = InvalidCompressionMethod;

if (nullness == BOOTCOL_NULL_FORCE_NOT_NULL)
{
Expand Down
8 changes: 4 additions & 4 deletions src/backend/catalog/genbki.pl
View file
Expand Up @@ -897,8 +897,11 @@ sub morph_row_for_pgattr
$row->{atttypid} = $type->{oid};
$row->{attlen} = $type->{typlen};
$row->{attbyval} = $type->{typbyval};
$row->{attstorage} = $type->{typstorage};
$row->{attalign} = $type->{typalign};
$row->{attstorage} = $type->{typstorage};

$row->{attcompression} =
$type->{typstorage} ne 'p' && $type->{typstorage} ne 'e' ? 'p' : '\0';

# set attndims if it's an array type
$row->{attndims} = $type->{typcategory} eq 'A' ? '1' : '0';
Expand All @@ -907,9 +910,6 @@ sub morph_row_for_pgattr
$row->{attcollation} =
$type->{typcollation} ne '0' ? $C_COLLATION_OID : 0;

$row->{attcompression} =
$type->{typstorage} ne 'p' && $type->{typstorage} ne 'e' ? 'p' : '\0';

if (defined $attr->{forcenotnull})
{
$row->{attnotnull} = 't';
Expand Down
19 changes: 10 additions & 9 deletions src/backend/catalog/heap.c
View file
Expand Up @@ -146,7 +146,8 @@ static Node *cookConstraint(ParseState *pstate,
/*
* The initializers below do not include trailing variable length fields,
* but that's OK - we're never going to reference anything beyond the
* fixed-size portion of the structure anyway.
* fixed-size portion of the structure anyway. Fields that can default
* to zeroes are also not mentioned.
*/

static const FormData_pg_attribute a1 = {
Expand All @@ -157,8 +158,8 @@ static const FormData_pg_attribute a1 = {
.attcacheoff = -1,
.atttypmod = -1,
.attbyval = false,
.attstorage = TYPSTORAGE_PLAIN,
.attalign = TYPALIGN_SHORT,
.attstorage = TYPSTORAGE_PLAIN,
.attnotnull = true,
.attislocal = true,
};
Expand All @@ -171,8 +172,8 @@ static const FormData_pg_attribute a2 = {
.attcacheoff = -1,
.atttypmod = -1,
.attbyval = true,
.attstorage = TYPSTORAGE_PLAIN,
.attalign = TYPALIGN_INT,
.attstorage = TYPSTORAGE_PLAIN,
.attnotnull = true,
.attislocal = true,
};
Expand All @@ -185,8 +186,8 @@ static const FormData_pg_attribute a3 = {
.attcacheoff = -1,
.atttypmod = -1,
.attbyval = true,
.attstorage = TYPSTORAGE_PLAIN,
.attalign = TYPALIGN_INT,
.attstorage = TYPSTORAGE_PLAIN,
.attnotnull = true,
.attislocal = true,
};
Expand All @@ -199,8 +200,8 @@ static const FormData_pg_attribute a4 = {
.attcacheoff = -1,
.atttypmod = -1,
.attbyval = true,
.attstorage = TYPSTORAGE_PLAIN,
.attalign = TYPALIGN_INT,
.attstorage = TYPSTORAGE_PLAIN,
.attnotnull = true,
.attislocal = true,
};
Expand All @@ -213,8 +214,8 @@ static const FormData_pg_attribute a5 = {
.attcacheoff = -1,
.atttypmod = -1,
.attbyval = true,
.attstorage = TYPSTORAGE_PLAIN,
.attalign = TYPALIGN_INT,
.attstorage = TYPSTORAGE_PLAIN,
.attnotnull = true,
.attislocal = true,
};
Expand All @@ -233,8 +234,8 @@ static const FormData_pg_attribute a6 = {
.attcacheoff = -1,
.atttypmod = -1,
.attbyval = true,
.attstorage = TYPSTORAGE_PLAIN,
.attalign = TYPALIGN_INT,
.attstorage = TYPSTORAGE_PLAIN,
.attnotnull = true,
.attislocal = true,
};
Expand Down Expand Up @@ -779,8 +780,9 @@ InsertPgAttributeTuples(Relation pg_attribute_rel,
slot[slotCount]->tts_values[Anum_pg_attribute_attcacheoff - 1] = Int32GetDatum(-1);
slot[slotCount]->tts_values[Anum_pg_attribute_atttypmod - 1] = Int32GetDatum(attrs->atttypmod);
slot[slotCount]->tts_values[Anum_pg_attribute_attbyval - 1] = BoolGetDatum(attrs->attbyval);
slot[slotCount]->tts_values[Anum_pg_attribute_attstorage - 1] = CharGetDatum(attrs->attstorage);
slot[slotCount]->tts_values[Anum_pg_attribute_attalign - 1] = CharGetDatum(attrs->attalign);
slot[slotCount]->tts_values[Anum_pg_attribute_attstorage - 1] = CharGetDatum(attrs->attstorage);
slot[slotCount]->tts_values[Anum_pg_attribute_attcompression - 1] = CharGetDatum(attrs->attcompression);
slot[slotCount]->tts_values[Anum_pg_attribute_attnotnull - 1] = BoolGetDatum(attrs->attnotnull);
slot[slotCount]->tts_values[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(attrs->atthasdef);
slot[slotCount]->tts_values[Anum_pg_attribute_atthasmissing - 1] = BoolGetDatum(attrs->atthasmissing);
Expand All @@ -790,7 +792,6 @@ InsertPgAttributeTuples(Relation pg_attribute_rel,
slot[slotCount]->tts_values[Anum_pg_attribute_attislocal - 1] = BoolGetDatum(attrs->attislocal);
slot[slotCount]->tts_values[Anum_pg_attribute_attinhcount - 1] = Int32GetDatum(attrs->attinhcount);
slot[slotCount]->tts_values[Anum_pg_attribute_attcollation - 1] = ObjectIdGetDatum(attrs->attcollation);
slot[slotCount]->tts_values[Anum_pg_attribute_attcompression - 1] = CharGetDatum(attrs->attcompression);
if (attoptions && attoptions[natts] != (Datum) 0)
slot[slotCount]->tts_values[Anum_pg_attribute_attoptions - 1] = attoptions[natts];
else
Expand Down
10 changes: 6 additions & 4 deletions src/backend/catalog/index.c
View file
Expand Up @@ -345,8 +345,8 @@ ConstructTupleDescriptor(Relation heapRelation,
to->attndims = from->attndims;
to->atttypmod = from->atttypmod;
to->attbyval = from->attbyval;
to->attstorage = from->attstorage;
to->attalign = from->attalign;
to->attstorage = from->attstorage;
to->attcompression = from->attcompression;
}
else
Expand All @@ -373,16 +373,16 @@ ConstructTupleDescriptor(Relation heapRelation,
*/
to->atttypid = keyType;
to->attlen = typeTup->typlen;
to->atttypmod = exprTypmod(indexkey);
to->attbyval = typeTup->typbyval;
to->attstorage = typeTup->typstorage;
to->attalign = typeTup->typalign;
to->atttypmod = exprTypmod(indexkey);
to->attstorage = typeTup->typstorage;

/*
* For expression columns, set attcompression invalid, since
* there's no table column from which to copy the value. Whenever
* we actually need to compress a value, we'll use whatever the
* current value of default_compression_method is at that point in
* current value of default_toast_compression is at that point in
* time.
*/
to->attcompression = InvalidCompressionMethod;
Expand Down Expand Up @@ -464,6 +464,8 @@ ConstructTupleDescriptor(Relation heapRelation,
to->attbyval = typeTup->typbyval;
to->attalign = typeTup->typalign;
to->attstorage = typeTup->typstorage;
/* As above, use the default compression method in this case */
to->attcompression = InvalidCompressionMethod;

ReleaseSysCache(tuple);
}
Expand Down
28 changes: 12 additions & 16 deletions src/backend/commands/tablecmds.c
View file
Expand Up @@ -2640,8 +2640,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
def->constraints = NIL;
def->location = -1;
if (CompressionMethodIsValid(attribute->attcompression))
def->compression = pstrdup(GetCompressionMethodName(
attribute->attcompression));
def->compression =
pstrdup(GetCompressionMethodName(attribute->attcompression));
else
def->compression = NULL;
inhSchema = lappend(inhSchema, def);
Expand Down Expand Up @@ -6596,12 +6596,19 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
attribute.atttypid = typeOid;
attribute.attstattarget = (newattnum > 0) ? -1 : 0;
attribute.attlen = tform->typlen;
attribute.atttypmod = typmod;
attribute.attnum = newattnum;
attribute.attbyval = tform->typbyval;
attribute.attndims = list_length(colDef->typeName->arrayBounds);
attribute.attstorage = tform->typstorage;
attribute.atttypmod = typmod;
attribute.attbyval = tform->typbyval;
attribute.attalign = tform->typalign;
attribute.attstorage = tform->typstorage;
/* do not set compression in views etc */
if (rel->rd_rel->relkind == RELKIND_RELATION ||
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
attribute.attcompression = GetAttributeCompression(&attribute,
colDef->compression);
else
attribute.attcompression = InvalidCompressionMethod;
attribute.attnotnull = colDef->is_not_null;
attribute.atthasdef = false;
attribute.atthasmissing = false;
Expand All @@ -6612,17 +6619,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
attribute.attinhcount = colDef->inhcount;
attribute.attcollation = collOid;

/*
* lookup attribute's compression method and store it in the
* attr->attcompression.
*/
if (rel->rd_rel->relkind == RELKIND_RELATION ||
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
attribute.attcompression = GetAttributeCompression(&attribute,
colDef->compression);
else
attribute.attcompression = InvalidCompressionMethod;

/* attribute.attacl is handled by InsertPgAttributeTuples() */

ReleaseSysCache(typeTuple);
Expand Down

0 comments on commit f5024d8

Please sign in to comment.