From 5b6d13eec72b960eb0f78542199380e49c8583d4 Mon Sep 17 00:00:00 2001 From: Simon Riggs Date: Wed, 6 Sep 2017 13:46:01 -0700 Subject: [PATCH] Allow SET STATISTICS on expression indexes Index columns are referenced by ordinal number rather than name, e.g. CREATE INDEX coord_idx ON measured (x, y, (z + t)); ALTER INDEX coord_idx ALTER COLUMN 3 SET STATISTICS 1000; Incompatibility note for release notes: \d+ for indexes now also displays Stats Target Authors: Alexander Korotkov, with contribution by Adrien NAYRAT Review: Adrien NAYRAT, Simon Riggs Wordsmith: Simon Riggs --- doc/src/sgml/ref/alter_index.sgml | 39 +++++++++++++++ src/backend/commands/tablecmds.c | 55 +++++++++++++++++----- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 16 +++++++ src/backend/utils/cache/syscache.c | 46 ++++++++++++++++++ src/bin/psql/describe.c | 2 + src/bin/psql/tab-complete.c | 5 +- src/include/nodes/parsenodes.h | 2 + src/include/utils/syscache.h | 3 ++ src/test/regress/expected/alter_table.out | 24 ++++++++++ src/test/regress/expected/create_index.out | 8 ++-- src/test/regress/sql/alter_table.sql | 16 +++++++ 13 files changed, 201 insertions(+), 17 deletions(-) diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml index ad77b5743a..7d6553d2db 100644 --- a/doc/src/sgml/ref/alter_index.sgml +++ b/doc/src/sgml/ref/alter_index.sgml @@ -26,6 +26,8 @@ ALTER INDEX [ IF EXISTS ] name SET ALTER INDEX name DEPENDS ON EXTENSION extension_name ALTER INDEX [ IF EXISTS ] name SET ( storage_parameter = value [, ... ] ) ALTER INDEX [ IF EXISTS ] name RESET ( storage_parameter [, ... ] ) +ALTER INDEX [ IF EXISTS ] name ALTER [ COLUMN ] column_number + SET STATISTICS integer ALTER INDEX ALL IN TABLESPACE name [ OWNED BY role_name [, ... ] ] SET TABLESPACE new_tablespace [ NOWAIT ] @@ -110,6 +112,25 @@ ALTER INDEX ALL IN TABLESPACE name + + ALTER [ COLUMN ] column_number SET STATISTICS integer + + + This form sets the per-column statistics-gathering target for + subsequent operations, though can + be used only on index columns that are defined as an expression. + Since expressions lack a unique name, we refer to them using the + ordinal number of the index column. + The target can be set in the range 0 to 10000; alternatively, set it + to -1 to revert to using the system default statistics + target (). + For more information on the use of statistics by the + PostgreSQL query planner, refer to + . + + + + @@ -130,6 +151,16 @@ ALTER INDEX ALL IN TABLESPACE name + + column_number + + + The ordinal number refers to the ordinal (left-to-right) position + of the index column. + + + + name @@ -235,6 +266,14 @@ ALTER INDEX distributors SET (fillfactor = 75); REINDEX INDEX distributors; + + Set the statistics-gathering target for an expression index: + +CREATE INDEX coord_idx ON measured (x, y, (z + t)); +ALTER INDEX coord_idx ALTER COLUMN 3 SET STATISTICS 1000; + + + diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0f08245a67..c8fc9cb7fe 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -375,9 +375,9 @@ static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName, static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmode); static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode); -static void ATPrepSetStatistics(Relation rel, const char *colName, +static void ATPrepSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode); -static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, +static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode); static ObjectAddress ATExecSetOptions(Relation rel, const char *colName, Node *options, bool isReset, LOCKMODE lockmode); @@ -3525,7 +3525,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */ ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* Performs own permission checks */ - ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode); + ATPrepSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode); pass = AT_PASS_MISC; break; case AT_SetOptions: /* ALTER COLUMN SET ( options ) */ @@ -3848,7 +3848,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, address = ATExecSetNotNull(tab, rel, cmd->name, lockmode); break; case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */ - address = ATExecSetStatistics(rel, cmd->name, cmd->def, lockmode); + address = ATExecSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode); break; case AT_SetOptions: /* ALTER COLUMN SET ( options ) */ address = ATExecSetOptions(rel, cmd->name, cmd->def, false, lockmode); @@ -6120,7 +6120,7 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE * ALTER TABLE ALTER COLUMN SET STATISTICS */ static void -ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode) +ATPrepSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode) { /* * We do our own permission checking because (a) we want to allow SET @@ -6138,6 +6138,15 @@ ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE errmsg("\"%s\" is not a table, materialized view, index, or foreign table", RelationGetRelationName(rel)))); + /* + * We allow referencing columns by numbers only for indexes, since + * table column numbers could contain gaps if columns are later dropped. + */ + if (rel->rd_rel->relkind != RELKIND_INDEX && !colName) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot refer to non-index column by number"))); + /* Permissions checks */ if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, @@ -6148,7 +6157,7 @@ ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE * Return value is the address of the modified column */ static ObjectAddress -ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode) +ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode) { int newtarget; Relation attrelation; @@ -6181,13 +6190,27 @@ ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE attrelation = heap_open(AttributeRelationId, RowExclusiveLock); - tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName); + if (colName) + { + tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName); + + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + colName, RelationGetRelationName(rel)))); + } + else + { + tuple = SearchSysCacheCopyAttNum(RelationGetRelid(rel), colNum); + + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column number %d of relation \"%s\" does not exist", + colNum, RelationGetRelationName(rel)))); + } - 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); attnum = attrtuple->attnum; @@ -6197,6 +6220,14 @@ ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE errmsg("cannot alter system column \"%s\"", colName))); + if (rel->rd_rel->relkind == RELKIND_INDEX && + rel->rd_index->indkey.values[attnum - 1] != 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter statistics on non-expression column \"%s\" of index \"%s\"", + NameStr(attrtuple->attname), RelationGetRelationName(rel)), + errhint("Alter statistics on table column instead."))); + attrtuple->attstattarget = newtarget; CatalogTupleUpdate(attrelation, &tuple->t_self, tuple); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index f9ddf4ed76..9bae2647fd 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3087,6 +3087,7 @@ _copyAlterTableCmd(const AlterTableCmd *from) COPY_SCALAR_FIELD(subtype); COPY_STRING_FIELD(name); + COPY_SCALAR_FIELD(num); COPY_NODE_FIELD(newowner); COPY_NODE_FIELD(def); COPY_SCALAR_FIELD(behavior); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 8d92c03633..11731da80a 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1098,6 +1098,7 @@ _equalAlterTableCmd(const AlterTableCmd *a, const AlterTableCmd *b) { COMPARE_SCALAR_FIELD(subtype); COMPARE_STRING_FIELD(name); + COMPARE_SCALAR_FIELD(num); COMPARE_NODE_FIELD(newowner); COMPARE_NODE_FIELD(def); COMPARE_SCALAR_FIELD(behavior); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 7d0de99baf..5eb398118e 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -2078,6 +2078,22 @@ alter_table_cmd: n->def = (Node *) makeInteger($6); $$ = (Node *)n; } + /* ALTER TABLE ALTER [COLUMN] SET STATISTICS */ + | ALTER opt_column Iconst SET STATISTICS SignedIconst + { + AlterTableCmd *n = makeNode(AlterTableCmd); + + if ($3 <= 0 || $3 > PG_INT16_MAX) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("column number must be in range from 1 to %d", PG_INT16_MAX), + parser_errposition(@3))); + + n->subtype = AT_SetStatistics; + n->num = (int16) $3; + n->def = (Node *) makeInteger($6); + $$ = (Node *)n; + } /* ALTER TABLE ALTER [COLUMN] SET ( column_parameter = value [, ... ] ) */ | ALTER opt_column ColId SET reloptions { diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c index 607fe9db79..fcbb683a99 100644 --- a/src/backend/utils/cache/syscache.c +++ b/src/backend/utils/cache/syscache.c @@ -1256,6 +1256,52 @@ SearchSysCacheExistsAttName(Oid relid, const char *attname) } +/* + * SearchSysCacheAttNum + * + * This routine is equivalent to SearchSysCache on the ATTNUM cache, + * except that it will return NULL if the found attribute is marked + * attisdropped. This is convenient for callers that want to act as + * though dropped attributes don't exist. + */ +HeapTuple +SearchSysCacheAttNum(Oid relid, int16 attnum) +{ + HeapTuple tuple; + + tuple = SearchSysCache2(ATTNUM, + ObjectIdGetDatum(relid), + Int16GetDatum(attnum)); + if (!HeapTupleIsValid(tuple)) + return NULL; + if (((Form_pg_attribute) GETSTRUCT(tuple))->attisdropped) + { + ReleaseSysCache(tuple); + return NULL; + } + return tuple; +} + +/* + * SearchSysCacheCopyAttNum + * + * As above, an attisdropped-aware version of SearchSysCacheCopy. + */ +HeapTuple +SearchSysCacheCopyAttNum(Oid relid, int16 attnum) +{ + HeapTuple tuple, + newtuple; + + tuple = SearchSysCacheAttNum(relid, attnum); + if (!HeapTupleIsValid(tuple)) + return NULL; + newtuple = heap_copytuple(tuple); + ReleaseSysCache(tuple); + return newtuple; +} + + /* * SysCacheGetAttr * diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index f6049cc9e5..6fb9bdd063 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -1742,6 +1742,7 @@ describeOneTableDetails(const char *schemaname, { headers[cols++] = gettext_noop("Storage"); if (tableinfo.relkind == RELKIND_RELATION || + tableinfo.relkind == RELKIND_INDEX || tableinfo.relkind == RELKIND_MATVIEW || tableinfo.relkind == RELKIND_FOREIGN_TABLE || tableinfo.relkind == RELKIND_PARTITIONED_TABLE) @@ -1841,6 +1842,7 @@ describeOneTableDetails(const char *schemaname, /* Statistics target, if the relkind supports this feature */ if (tableinfo.relkind == RELKIND_RELATION || + tableinfo.relkind == RELKIND_INDEX || tableinfo.relkind == RELKIND_MATVIEW || tableinfo.relkind == RELKIND_FOREIGN_TABLE || tableinfo.relkind == RELKIND_PARTITIONED_TABLE) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 7959f9ac16..2ab8809fa5 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1644,7 +1644,10 @@ psql_completion(const char *text, int start, int end) "UNION SELECT 'ALL IN TABLESPACE'"); /* ALTER INDEX */ else if (Matches3("ALTER", "INDEX", MatchAny)) - COMPLETE_WITH_LIST4("OWNER TO", "RENAME TO", "SET", "RESET"); + COMPLETE_WITH_LIST5("ALTER COLUMN", "OWNER TO", "RENAME TO", "SET", "RESET"); + /* ALTER INDEX ALTER COLUMN */ + else if (Matches6("ALTER", "INDEX", MatchAny, "ALTER", "COLUMN", MatchAny)) + COMPLETE_WITH_CONST("SET STATISTICS"); /* ALTER INDEX SET */ else if (Matches4("ALTER", "INDEX", MatchAny, "SET")) COMPLETE_WITH_LIST2("(", "TABLESPACE"); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index ef6753e31a..3171815320 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1777,6 +1777,8 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ AlterTableType subtype; /* Type of table alteration to apply */ char *name; /* column, constraint, or trigger to act on, * or tablespace */ + int16 num; /* attribute number for columns referenced + * by number */ RoleSpec *newowner; Node *def; /* definition of new column, index, * constraint, or parent table */ diff --git a/src/include/utils/syscache.h b/src/include/utils/syscache.h index 8352b40f4e..8a92ea27ac 100644 --- a/src/include/utils/syscache.h +++ b/src/include/utils/syscache.h @@ -131,6 +131,9 @@ extern HeapTuple SearchSysCacheAttName(Oid relid, const char *attname); extern HeapTuple SearchSysCacheCopyAttName(Oid relid, const char *attname); extern bool SearchSysCacheExistsAttName(Oid relid, const char *attname); +extern HeapTuple SearchSysCacheAttNum(Oid relid, int16 attnum); +extern HeapTuple SearchSysCacheCopyAttNum(Oid relid, int16 attnum); + extern Datum SysCacheGetAttr(int cacheId, HeapTuple tup, AttrNumber attributeNumber, bool *isNull); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index ed03cb9c63..0f36423163 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -94,6 +94,30 @@ SELECT * FROM tmp; | 4 | name | text | 4.1 | 4.1 | 2 | ((4.1,4.1),(3.1,3.1)) | Mon May 01 00:30:30 1995 PDT | c | {"Mon May 01 00:30:30 1995 PDT","Mon Aug 24 14:43:07 1992 PDT","Wed Dec 31 16:00:00 1969 PST"} | 314159 | (1,1) | 512 | 1 2 3 4 5 6 7 8 | magnetic disk | (1.1,1.1) | [(4.1,4.1),(3.1,3.1)] | ((0,2),(4.1,4.1),(3.1,3.1)) | (4.1,4.1),(3.1,3.1) | ["Wed Dec 31 16:00:00 1969 PST" "infinity"] | Thu Jan 01 00:00:00 1970 | @ 1 hour 10 secs | {1,2,3,4} | {1,2,3,4} | {1,2,3,4} (1 row) +CREATE INDEX tmp_idx ON tmp (a, (d + e), b); +ALTER INDEX tmp_idx ALTER COLUMN 0 SET STATISTICS 1000; +ERROR: column number must be in range from 1 to 32767 +LINE 1: ALTER INDEX tmp_idx ALTER COLUMN 0 SET STATISTICS 1000; + ^ +ALTER INDEX tmp_idx ALTER COLUMN 1 SET STATISTICS 1000; +ERROR: cannot alter statistics on non-expression column "a" of index "tmp_idx" +HINT: Alter statistics on table column instead. +ALTER INDEX tmp_idx ALTER COLUMN 2 SET STATISTICS 1000; +\d+ tmp_idx + Index "public.tmp_idx" + Column | Type | Definition | Storage | Stats target +--------+------------------+------------+---------+-------------- + a | integer | a | plain | + expr | double precision | (d + e) | plain | 1000 + b | cstring | b | plain | +btree, for table "public.tmp" + +ALTER INDEX tmp_idx ALTER COLUMN 3 SET STATISTICS 1000; +ERROR: cannot alter statistics on non-expression column "b" of index "tmp_idx" +HINT: Alter statistics on table column instead. +ALTER INDEX tmp_idx ALTER COLUMN 4 SET STATISTICS 1000; +ERROR: column number 4 of relation "tmp_idx" does not exist +ALTER INDEX tmp_idx ALTER COLUMN 2 SET STATISTICS -1; DROP TABLE tmp; -- -- rename - check on both non-temp and temp tables diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 064adb4640..8450f2463e 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2324,10 +2324,10 @@ DROP TABLE array_gin_test; CREATE INDEX gin_relopts_test ON array_index_op_test USING gin (i) WITH (FASTUPDATE=on, GIN_PENDING_LIST_LIMIT=128); \d+ gin_relopts_test - Index "public.gin_relopts_test" - Column | Type | Definition | Storage ---------+---------+------------+--------- - i | integer | i | plain + Index "public.gin_relopts_test" + Column | Type | Definition | Storage | Stats target +--------+---------+------------+---------+-------------- + i | integer | i | plain | gin, for table "public.array_index_op_test" Options: fastupdate=on, gin_pending_list_limit=128 diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 9a20dd141a..e6f6669880 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -142,6 +142,22 @@ INSERT INTO tmp (a, b, c, d, e, f, g, h, i, j, k, l, m, n, p, q, r, s, t, u, SELECT * FROM tmp; +CREATE INDEX tmp_idx ON tmp (a, (d + e), b); + +ALTER INDEX tmp_idx ALTER COLUMN 0 SET STATISTICS 1000; + +ALTER INDEX tmp_idx ALTER COLUMN 1 SET STATISTICS 1000; + +ALTER INDEX tmp_idx ALTER COLUMN 2 SET STATISTICS 1000; + +\d+ tmp_idx + +ALTER INDEX tmp_idx ALTER COLUMN 3 SET STATISTICS 1000; + +ALTER INDEX tmp_idx ALTER COLUMN 4 SET STATISTICS 1000; + +ALTER INDEX tmp_idx ALTER COLUMN 2 SET STATISTICS -1; + DROP TABLE tmp;