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