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 */

Reply via email to