On Tue, Dec 29, 2009 at 9:12 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Robert Haas <robertmh...@gmail.com> writes:
>> Yep.  It would also lower the barrier to future innovations of that
>> type, which would be a good thing, IMO.  Unfortunately we'd likely
>> need to continue to support the existing syntax at least for
>> attstattarget, which is kind of a bummer, but seems managable.  I
>> think we could throw over the syntax for ALTER TABLE ... ADD
>> STATISTICS DISTINCT since it is an 8.5-ism.
>
> Yeah --- if we think we might want to do this, now is the time,
> before we're stuck with supporting that syntax.  (I was thinking
> earlier today that attdistinct was already in 8.4, but it's not.)

[ Hack, hack, hack. ]

I'm not quite sure what the correct approach is to making attoptions
available to examine_attribute(), which can't get at them in any very
obvious way because the relation descriptor it gets passed as an
argument only includes the fixed-size portion of each pg_attribute
tuple.  The obvious approaches are:

1. Extract attrelid and attnum from the tuple and issue a syscache
lookup to get the full tuple.
2. Make RelationBuildTupleDesc() parse the attoptions and store them
into a new RelationData member.

I'm leaning toward the latter method.  The upside of that method is
that making attoptions part of the Relation descriptor means that
they'll be conveniently available to all clients of the relcache.  The
downside is that right now, only ANALYZE actually needs to care at the
moment, and yet we're incurring the overhead across the board.  My
thought is that that's OK, but I wonder if anyone thinks it might
cause a measurable performance hit?

WIP patch attached.  Right now this just adds the ability to set and
store attoptions, but doesn't actually do anything useful with them.
No doc changes, no pg_dump support, no psql support, doesn't remove
the SET STATISTICS DISTINCT code.  All these warts will be fixed in a
future version once I decide what to do about the problem mentioned
above.

...Robert
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 24480e3..596591c 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -21,6 +21,7 @@
 #include "access/reloptions.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
+#include "commands/tablecmds.h"
 #include "commands/tablespace.h"
 #include "nodes/makefuncs.h"
 #include "utils/array.h"
@@ -196,6 +197,22 @@ static relopt_real realRelOpts[] =
 		},
 		-1, 0.0, DBL_MAX
 	},
+	{
+		{
+			"n_distinct",
+			"Sets the planner's estimate of the number of distinct values appearing in a column (excluding child relations).",
+			RELOPT_KIND_ATTRIBUTE
+		},
+		0, -1.0, DBL_MAX
+	},
+	{
+		{
+			"n_distinct_inherited",
+			"Sets the planner's estimate of the number of distinct values appearing in a column (including child relations).",
+			RELOPT_KIND_ATTRIBUTE
+		},
+		0, -1.0, DBL_MAX
+	},
 	/* list terminator */
 	{{NULL}}
 };
@@ -1187,6 +1204,37 @@ index_reloptions(RegProcedure amoptions, Datum reloptions, bool validate)
 }
 
 /*
+ * Option parser for attribute reloptions
+ */
+bytea *
+attribute_reloptions(Datum reloptions, bool validate)
+{
+	relopt_value *options;
+	AttributeOpts  *aopts;
+	int			numoptions;
+	static const relopt_parse_elt tab[] = {
+		{"n_distinct", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct)},
+		{"n_distinct_inherited", RELOPT_TYPE_REAL, offsetof(AttributeOpts, n_distinct_inherited)}
+	};
+
+	options = parseRelOptions(reloptions, validate, RELOPT_KIND_ATTRIBUTE,
+							  &numoptions);
+
+	/* if none set, we're done */
+	if (numoptions == 0)
+		return NULL;
+
+	aopts = allocateReloptStruct(sizeof(AttributeOpts), options, numoptions);
+
+	fillRelOptions((void *) aopts, sizeof(AttributeOpts), options, numoptions,
+				   validate, tab, lengthof(tab));
+
+	pfree(options);
+
+	return (bytea *) aopts;
+}
+
+/*
  * Option parser for tablespace reloptions
  */
 bytea *
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index b54520f..8acdde8 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -362,7 +362,7 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
 			return false;
 		if (attr1->attinhcount != attr2->attinhcount)
 			return false;
-		/* attacl is ignored, since it's not even present... */
+		/* attacl and attoptions are not even present... */
 	}
 
 	if (tupdesc1->constr != NULL)
@@ -479,7 +479,7 @@ TupleDescInitEntry(TupleDesc desc,
 	att->attisdropped = false;
 	att->attislocal = true;
 	att->attinhcount = 0;
-	/* attacl is not set because it's not present in tupledescs */
+	/* attacl and attoptions are not present in tupledescs */
 
 	tuple = SearchSysCache(TYPEOID,
 						   ObjectIdGetDatum(oidtypeid),
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 7a51b09..35a13e4 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -351,7 +351,8 @@ sub emit_pgattr_row
         attisdropped  => 'f',
         attislocal    => 't',
         attinhcount   => '0',
-        attacl        => '_null_'
+        attacl        => '_null_',
+        attoptions    => '_null_'
     );
     return {%PGATTR_DEFAULTS, %row};
 }
@@ -378,6 +379,7 @@ sub emit_schemapg_row
     $row->{attstorage}  = q|'|  . $row->{attstorage} . q|'|;
     $row->{attalign}    = q|'|  . $row->{attalign}   . q|'|;
     $row->{attacl}      = q|{ 0 }|;
+    $row->{attoptions}  = q|{ 0 }|;
 
     # Expand booleans from 'f'/'t' to 'false'/'true'.
     # Some values might be other macros (eg FLOAT4PASSBYVAL), don't change.
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ce890e1..085d945 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -116,37 +116,37 @@ static List *insert_ordered_unique_oid(List *list, Oid datum);
 static FormData_pg_attribute a1 = {
 	0, {"ctid"}, TIDOID, 0, 0, sizeof(ItemPointerData),
 	SelfItemPointerAttributeNumber, 0, -1, -1,
-	false, 'p', 's', true, false, false, true, 0, {0}
+	false, 'p', 's', true, false, false, true, 0, {0}, {0}
 };
 
 static FormData_pg_attribute a2 = {
 	0, {"oid"}, OIDOID, 0, 0, sizeof(Oid),
 	ObjectIdAttributeNumber, 0, -1, -1,
-	true, 'p', 'i', true, false, false, true, 0, {0}
+	true, 'p', 'i', true, false, false, true, 0, {0}, {0}
 };
 
 static FormData_pg_attribute a3 = {
 	0, {"xmin"}, XIDOID, 0, 0, sizeof(TransactionId),
 	MinTransactionIdAttributeNumber, 0, -1, -1,
-	true, 'p', 'i', true, false, false, true, 0, {0}
+	true, 'p', 'i', true, false, false, true, 0, {0}, {0}
 };
 
 static FormData_pg_attribute a4 = {
 	0, {"cmin"}, CIDOID, 0, 0, sizeof(CommandId),
 	MinCommandIdAttributeNumber, 0, -1, -1,
-	true, 'p', 'i', true, false, false, true, 0, {0}
+	true, 'p', 'i', true, false, false, true, 0, {0}, {0}
 };
 
 static FormData_pg_attribute a5 = {
 	0, {"xmax"}, XIDOID, 0, 0, sizeof(TransactionId),
 	MaxTransactionIdAttributeNumber, 0, -1, -1,
-	true, 'p', 'i', true, false, false, true, 0, {0}
+	true, 'p', 'i', true, false, false, true, 0, {0}, {0}
 };
 
 static FormData_pg_attribute a6 = {
 	0, {"cmax"}, CIDOID, 0, 0, sizeof(CommandId),
 	MaxCommandIdAttributeNumber, 0, -1, -1,
-	true, 'p', 'i', true, false, false, true, 0, {0}
+	true, 'p', 'i', true, false, false, true, 0, {0}, {0}
 };
 
 /*
@@ -519,8 +519,9 @@ InsertPgAttributeTuple(Relation pg_attribute_rel,
 	values[Anum_pg_attribute_attislocal - 1] = BoolGetDatum(new_attribute->attislocal);
 	values[Anum_pg_attribute_attinhcount - 1] = Int32GetDatum(new_attribute->attinhcount);
 
-	/* start out with empty permissions */
+	/* start out with empty permissions and empty options */
 	nulls[Anum_pg_attribute_attacl - 1] = true;
+	nulls[Anum_pg_attribute_attoptions - 1] = true;
 
 	tup = heap_form_tuple(RelationGetDescr(pg_attribute_rel), values, nulls);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 75f65ba..54f321f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -288,6 +288,10 @@ static void ATPrepSetDistinct(Relation rel, const char *colName,
 					Node *newValue);
 static void ATExecSetDistinct(Relation rel, const char *colName,
 				 Node *newValue);
+static void ATPrepSetOptions(Relation rel, const char *colName,
+				 Node *options);
+static void ATExecSetOptions(Relation rel, const char *colName,
+				 Node *options, bool isReset);
 static void ATExecSetStorage(Relation rel, const char *colName,
 				 Node *newValue);
 static void ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
@@ -2431,6 +2435,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			ATPrepSetDistinct(rel, cmd->name, cmd->def);
 			pass = AT_PASS_COL_ATTRS;
 			break;
+		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
+		case AT_ResetOptions:	/* ALTER COLUMN RESET ( options ) */
+			ATSimpleRecursion(wqueue, rel, cmd, recurse);
+			/* Performs own permission checks */
+			ATPrepSetOptions(rel, cmd->name, cmd->def);
+			pass = AT_PASS_COL_ATTRS;
+			break;
 		case AT_SetStorage:		/* ALTER COLUMN SET STORAGE */
 			ATSimplePermissions(rel, false);
 			ATSimpleRecursion(wqueue, rel, cmd, recurse);
@@ -2648,6 +2659,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		case AT_SetDistinct:	/* ALTER COLUMN SET STATISTICS DISTINCT */
 			ATExecSetDistinct(rel, cmd->name, cmd->def);
 			break;
+		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
+			ATExecSetOptions(rel, cmd->name, cmd->def, false);
+			break;
+		case AT_ResetOptions:	/* ALTER COLUMN RESET ( options ) */
+			ATExecSetOptions(rel, cmd->name, cmd->def, true);
+			break;
 		case AT_SetStorage:		/* ALTER COLUMN SET STORAGE */
 			ATExecSetStorage(rel, cmd->name, cmd->def);
 			break;
@@ -4177,6 +4194,31 @@ ATPrepSetDistinct(Relation rel, const char *colName, Node *newValue)
 					   RelationGetRelationName(rel));
 }
 
+/*
+ * ALTER TABLE ALTER COLUMN SET ( options )
+ * ALTER TABLE ALTER COLUMN RESET ( options )
+ */
+static void
+ATPrepSetOptions(Relation rel, const char *colName, Node *options)
+{
+	/*
+	 * We do our own permission checking because (a) we want to allow options
+	 * to be set on indexes, and (b) we want to allow options to be set on
+	 * system catalogs without requiring allowSystemTableMods to be turned on.
+	 */
+	if (rel->rd_rel->relkind != RELKIND_RELATION &&
+		rel->rd_rel->relkind != RELKIND_INDEX)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("\"%s\" is not a table or index",
+						RelationGetRelationName(rel))));
+
+	/* Permissions checks */
+	if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+					   RelationGetRelationName(rel));
+}
+
 static void
 ATExecSetDistinct(Relation rel, const char *colName, Node *newValue)
 {
@@ -4240,6 +4282,67 @@ ATExecSetDistinct(Relation rel, const char *colName, Node *newValue)
 	heap_close(attrelation, RowExclusiveLock);
 }
 
+static void
+ATExecSetOptions(Relation rel, const char *colName, Node *options,
+				 bool isReset)
+{
+	Relation	attrelation;
+	HeapTuple	tuple,
+				newtuple;
+	Form_pg_attribute attrtuple;
+	Datum		datum,
+				newOptions;
+	bool		isnull;
+	Datum		repl_val[Natts_pg_attribute];
+	bool		repl_null[Natts_pg_attribute];
+	bool		repl_repl[Natts_pg_attribute];
+
+	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
+
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_COLUMN),
+				 errmsg("column \"%s\" of relation \"%s\" does not exist",
+						colName, RelationGetRelationName(rel))));
+	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
+
+	if (attrtuple->attnum <= 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));
+
+	/* Generate new proposed attoptions (text array) */
+	Assert(IsA(options, List));
+	datum = SysCacheGetAttr(ATTNAME, tuple, Anum_pg_attribute_attoptions,
+		&isnull);
+	newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
+									 (List *) options, NULL, NULL, false,
+									 isReset);
+	(void) attribute_reloptions(newOptions, true);
+
+	/* Build new tuple. */
+	memset(repl_null, false, sizeof(repl_null));
+	memset(repl_repl, false, sizeof(repl_repl));
+	if (newOptions != (Datum) 0)
+		repl_val[Anum_pg_attribute_attoptions - 1] = newOptions;
+	else
+		repl_null[Anum_pg_attribute_attoptions - 1] = true;
+	repl_repl[Anum_pg_attribute_attoptions - 1] = true;
+	newtuple = heap_modify_tuple(tuple, RelationGetDescr(attrelation),
+								 repl_val, repl_null, repl_repl);
+	ReleaseSysCache(tuple);
+
+	/* Update system catalog. */
+	simple_heap_update(attrelation, &newtuple->t_self, newtuple);
+	CatalogUpdateIndexes(attrelation, newtuple);
+	heap_freetuple(newtuple);
+
+	heap_close(attrelation, RowExclusiveLock);
+}
+
 /*
  * ALTER TABLE ALTER COLUMN SET STORAGE
  */
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7aa387e..49fff9f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1654,6 +1654,24 @@ alter_table_cmd:
 					n->def = (Node *) $7;
 					$$ = (Node *)n;
 				}
+			/* ALTER TABLE <name> ALTER [COLUMN] <colname> SET ( column_parameter = value [, ... ] ) */
+			| ALTER opt_column ColId SET reloptions
+				{
+					AlterTableCmd *n = makeNode(AlterTableCmd);
+					n->subtype = AT_SetOptions;
+					n->name = $3;
+					n->def = (Node *) $5;
+					$$ = (Node *)n;
+				}
+			/* ALTER TABLE <name> ALTER [COLUMN] <colname> SET ( column_parameter = value [, ... ] ) */
+			| ALTER opt_column ColId RESET reloptions
+				{
+					AlterTableCmd *n = makeNode(AlterTableCmd);
+					n->subtype = AT_ResetOptions;
+					n->name = $3;
+					n->def = (Node *) $5;
+					$$ = (Node *)n;
+				}
 			/* ALTER TABLE <name> ALTER [COLUMN] <colname> SET STORAGE <storagemode> */
 			| ALTER opt_column ColId SET STORAGE ColId
 				{
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index a0087bc..db21390 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -40,7 +40,8 @@ typedef enum relopt_kind
 	RELOPT_KIND_HASH = (1 << 3),
 	RELOPT_KIND_GIN = (1 << 4),
 	RELOPT_KIND_GIST = (1 << 5),
-	RELOPT_KIND_TABLESPACE = (1 << 6),
+	RELOPT_KIND_ATTRIBUTE = (1 << 6),
+	RELOPT_KIND_TABLESPACE = (1 << 7),
 	/* if you add a new kind, make sure you update "last_default" too */
 	RELOPT_KIND_LAST_DEFAULT = RELOPT_KIND_TABLESPACE,
 	/* some compilers treat enums as signed ints, so we can't use 1 << 31 */
@@ -266,6 +267,7 @@ extern bytea *default_reloptions(Datum reloptions, bool validate,
 extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
 extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
 				 bool validate);
+extern bytea *attribute_reloptions(Datum reloptions, bool validate);
 extern bytea *tablespace_reloptions(Datum reloptions, bool validate);
 
 #endif   /* RELOPTIONS_H */
diff --git a/src/include/catalog/pg_attribute.h b/src/include/catalog/pg_attribute.h
index 6fdc6b8..998418b 100644
--- a/src/include/catalog/pg_attribute.h
+++ b/src/include/catalog/pg_attribute.h
@@ -157,6 +157,9 @@ CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(75) BK
 
 	/* Column-level access permissions */
 	aclitem		attacl[1];
+
+	/* Column-level options */
+	aclitem		attoptions[1];
 } FormData_pg_attribute;
 
 /*
@@ -180,7 +183,7 @@ typedef FormData_pg_attribute *Form_pg_attribute;
  * ----------------
  */
 
-#define Natts_pg_attribute				19
+#define Natts_pg_attribute				20
 #define Anum_pg_attribute_attrelid		1
 #define Anum_pg_attribute_attname		2
 #define Anum_pg_attribute_atttypid		3
@@ -200,6 +203,7 @@ typedef FormData_pg_attribute *Form_pg_attribute;
 #define Anum_pg_attribute_attislocal	17
 #define Anum_pg_attribute_attinhcount	18
 #define Anum_pg_attribute_attacl		19
+#define Anum_pg_attribute_attoptions	20
 
 
 /* ----------------
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index c5b15f5..2f40198 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -128,7 +128,7 @@ typedef FormData_pg_class *Form_pg_class;
 /* Note: "3" in the relfrozenxid column stands for FirstNormalTransactionId */
 DATA(insert OID = 1247 (  pg_type		PGNSP 71 PGUID 0 1247 0 0 0 0 0 f f f r 28 0 t f f f f f 3 _null_ _null_ ));
 DESCR("");
-DATA(insert OID = 1249 (  pg_attribute	PGNSP 75 PGUID 0 1249 0 0 0 0 0 f f f r 19 0 f f f f f f 3 _null_ _null_ ));
+DATA(insert OID = 1249 (  pg_attribute	PGNSP 75 PGUID 0 1249 0 0 0 0 0 f f f r 20 0 f f f f f f 3 _null_ _null_ ));
 DESCR("");
 DATA(insert OID = 1255 (  pg_proc		PGNSP 81 PGUID 0 1255 0 0 0 0 0 f f f r 25 0 t f f f f f 3 _null_ _null_ ));
 DESCR("");
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 8bc716a..276d1eb 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -17,6 +17,11 @@
 #include "nodes/parsenodes.h"
 #include "utils/relcache.h"
 
+typedef struct AttributeOpts
+{
+	float8		n_distinct;
+	float8		n_distinct_inherited;
+} AttributeOpts;
 
 extern Oid	DefineRelation(CreateStmt *stmt, char relkind);
 
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e05f93b..32ba917 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1102,6 +1102,8 @@ typedef enum AlterTableType
 	AT_SetNotNull,				/* alter column set not null */
 	AT_SetStatistics,			/* alter column set statistics */
 	AT_SetDistinct,				/* alter column set statistics distinct */
+	AT_SetOptions,				/* alter column set ( options ) */
+	AT_ResetOptions,			/* alter column reset ( options ) */
 	AT_SetStorage,				/* alter column set storage */
 	AT_DropColumn,				/* drop column */
 	AT_DropColumnRecurse,		/* internal to commands/tablecmds.c */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to