On 2/8/19, Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > [v2 patch]
I poked this around a bit and found that this mechanism only works for bootstrapped tables, as those are the only ones where we can scribble on pg_attribute entries directly during bootstrap. As such, with this patch we cannot perform ALTER TABLE for pg_index or pg_largeobject* [1]. IMHO, it's not worth it to introduce new notation unless it offers complete coverage. If we're willing to only solve the problem for pg_class and pg_attribute, I'd rather mark the table rather than the columns, because we already have visibility into CATALOG_VARLEN. (rough example attached) On 2/14/19, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > That already exists: 'm': Value can be stored compressed inline > > I agree that it seems we should be using that for those tables that > don't have a toast table. Maybe the genbki stuff could do it > automatically for the appropriate catalogs. The docs say: (Actually, out-of-line storage will still be performed for such columns, but only as a last resort when there is no other way to make the row small enough to fit on a page.) If we allow 'm' as an exception, would that interfere with this? My demo patch has this just in case: - if (att->attstorage != 'p') + if (att->attstorage != 'p' && + !(att->attstorage == 'm' && IsCatalogRelation(rel))) has_toastable_attrs = true; Here's another idea: During initdb, do "ALTER TABLE ALTER COLUMN xyz SET STORAGE MAIN;" In initdb, we already pass "-O" to allow system table mods, so I think we would have to just make sure this one statement doesn't try to add a toast table. We could have genbki.pl emit a file with SQL statements to cover all necessary tables/columns. [1] https://www.postgresql.org/message-id/20180928190630.crt43sk5zd5p555h%40alvherre.pgsql -- John Naylor https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm index 3bf308fe3b..885df79f92 100644 --- a/src/backend/catalog/Catalog.pm +++ b/src/backend/catalog/Catalog.pm @@ -124,6 +124,7 @@ sub ParseHeader $catalog{rowtype_oid_macro} = ''; } $catalog{schema_macro} = /BKI_SCHEMA_MACRO/ ? 1 : 0; + $catalog{no_toast} = /BKI_NO_TOAST/ ? 1 : 0; $declaring_attributes = 1; } elsif ($is_client_code) diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl index 4935e00fb2..b4777e5e5a 100644 --- a/src/backend/catalog/genbki.pl +++ b/src/backend/catalog/genbki.pl @@ -658,7 +658,7 @@ sub gen_pg_attribute $row{attnum} = $attnum; $row{attrelid} = $table->{relation_oid}; - morph_row_for_pgattr(\%row, $schema, $attr, $priornotnull); + morph_row_for_pgattr($table, \%row, $schema, $attr, $priornotnull); $priornotnull &= ($row{attnotnull} eq 't'); # If it's bootstrapped, put an entry in postgres.bki. @@ -691,7 +691,7 @@ sub gen_pg_attribute $row{attrelid} = $table->{relation_oid}; $row{attstattarget} = '0'; - morph_row_for_pgattr(\%row, $schema, $attr, 1); + morph_row_for_pgattr($table, \%row, $schema, $attr, 1); print_bki_insert(\%row, $schema); } } @@ -707,7 +707,7 @@ sub gen_pg_attribute # Any value not handled here must be supplied by caller. sub morph_row_for_pgattr { - my ($row, $pgattr_schema, $attr, $priornotnull) = @_; + my ($table, $row, $pgattr_schema, $attr, $priornotnull) = @_; my $attname = $attr->{name}; my $atttype = $attr->{type}; @@ -719,9 +719,17 @@ 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}; + if ($table->{no_toast} && $attr->{is_varlen}) + { + $row->{attstorage} = 'm'; + } + else + { + $row->{attstorage} = $type->{typstorage}; + } + # set attndims if it's an array type $row->{attndims} = $type->{typcategory} eq 'A' ? '1' : '0'; diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 77be19175a..37e494b205 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -18,6 +18,7 @@ #include "access/tuptoaster.h" #include "access/xact.h" #include "catalog/binary_upgrade.h" +#include "catalog/catalog.h" #include "catalog/dependency.h" #include "catalog/heap.h" #include "catalog/index.h" @@ -435,7 +436,8 @@ needs_toast_table(Relation rel) maxlength_unknown = true; else data_length += maxlen; - if (att->attstorage != 'p') + if (att->attstorage != 'p' && + !(att->attstorage == 'm' && IsCatalogRelation(rel))) has_toastable_attrs = true; } } diff --git a/src/include/catalog/genbki.h b/src/include/catalog/genbki.h index 1b8e4e9e19..1984bbc426 100644 --- a/src/include/catalog/genbki.h +++ b/src/include/catalog/genbki.h @@ -27,6 +27,7 @@ #define BKI_SHARED_RELATION #define BKI_ROWTYPE_OID(oid,oidmacro) #define BKI_SCHEMA_MACRO +#define BKI_NO_TOAST /* Options that may appear after an attribute (on the same line) */ #define BKI_FORCE_NULL diff --git a/src/include/catalog/pg_attribute.h b/src/include/catalog/pg_attribute.h index a6ec122389..66120244f5 100644 --- a/src/include/catalog/pg_attribute.h +++ b/src/include/catalog/pg_attribute.h @@ -34,7 +34,7 @@ * You may need to change catalog/genbki.pl as well. * ---------------- */ -CATALOG(pg_attribute,1249,AttributeRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(75,AttributeRelation_Rowtype_Id) BKI_SCHEMA_MACRO +CATALOG(pg_attribute,1249,AttributeRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(75,AttributeRelation_Rowtype_Id) BKI_SCHEMA_MACRO BKI_NO_TOAST { Oid attrelid; /* OID of relation containing this attribute */ NameData attname; /* name of attribute */ diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h index 5d82ce09a6..151b195f02 100644 --- a/src/include/catalog/pg_class.h +++ b/src/include/catalog/pg_class.h @@ -26,7 +26,7 @@ * typedef struct FormData_pg_class * ---------------- */ -CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,RelationRelation_Rowtype_Id) BKI_SCHEMA_MACRO +CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,RelationRelation_Rowtype_Id) BKI_SCHEMA_MACRO BKI_NO_TOAST { Oid oid; /* oid */ NameData relname; /* class name */