ALTER INDEX .. SET STATISTICS ... behaviour

Started by Alexander Korotkovover 8 years ago14 messages
#1Alexander Korotkov
a.korotkov@postgrespro.ru

Hackers,

I've discovered that PostgreSQL is able to run following kind of queries in
order to change statistic-gathering target for an indexed expression.

ALTER INDEX index_name ALTER COLUMN expr SET STATISTICS stat_target;

It's been previously discussed in [1].

I think this should be fixed not just in docs. This is why I've started
thread in pgsql-hackers. For me usage of internal column names "expr",
"expr1", "expr2" etc. looks weird. And I think we should replace it with a
better syntax. What do you think about these options?

ALTER INDEX index_name ALTER EXPRESSION 0 SET STATISTICS stat_target; --
Refer expression by its number
ALTER INDEX index_name ALTER EXPRESSION (x + y) SET STATISTICS stat_target;
-- Refer expression by its definition

1.
/messages/by-id/20150716143149.GO2301@postgresql.org

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alexander Korotkov (#1)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:

I've discovered that PostgreSQL is able to run following kind of queries in
order to change statistic-gathering target for an indexed expression.

ALTER INDEX index_name ALTER COLUMN expr SET STATISTICS stat_target;

It's been previously discussed in [1].

I think this should be fixed not just in docs. This is why I've started
thread in pgsql-hackers. For me usage of internal column names "expr",
"expr1", "expr2" etc. looks weird. And I think we should replace it with a
better syntax. What do you think about these options?

ALTER INDEX index_name ALTER EXPRESSION 0 SET STATISTICS stat_target; --
Refer expression by its number
ALTER INDEX index_name ALTER EXPRESSION (x + y) SET STATISTICS stat_target;
-- Refer expression by its definition

Don't like either of those particularly, but what about just targeting
a column by column number, independently of whether it's an expression?

ALTER INDEX myindex ALTER COLUMN 4 SET STATISTICS 1000;

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Tom Lane (#2)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On Wed, May 31, 2017 at 6:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:

I've discovered that PostgreSQL is able to run following kind of queries

in

order to change statistic-gathering target for an indexed expression.

ALTER INDEX index_name ALTER COLUMN expr SET STATISTICS stat_target;

It's been previously discussed in [1].

I think this should be fixed not just in docs. This is why I've started
thread in pgsql-hackers. For me usage of internal column names "expr",
"expr1", "expr2" etc. looks weird. And I think we should replace it

with a

better syntax. What do you think about these options?

ALTER INDEX index_name ALTER EXPRESSION 0 SET STATISTICS stat_target; --
Refer expression by its number
ALTER INDEX index_name ALTER EXPRESSION (x + y) SET STATISTICS

stat_target;

-- Refer expression by its definition

Don't like either of those particularly, but what about just targeting
a column by column number, independently of whether it's an expression?

ALTER INDEX myindex ALTER COLUMN 4 SET STATISTICS 1000;

I completely agree with that.
If no objections, I will write a patch for that.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#4Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Alexander Korotkov (#3)
1 attachment(s)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On Wed, May 31, 2017 at 7:18 PM, Alexander Korotkov <
a.korotkov@postgrespro.ru> wrote:

On Wed, May 31, 2017 at 6:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:

I've discovered that PostgreSQL is able to run following kind of

queries in

order to change statistic-gathering target for an indexed expression.

ALTER INDEX index_name ALTER COLUMN expr SET STATISTICS stat_target;

It's been previously discussed in [1].

I think this should be fixed not just in docs. This is why I've started
thread in pgsql-hackers. For me usage of internal column names "expr",
"expr1", "expr2" etc. looks weird. And I think we should replace it

with a

better syntax. What do you think about these options?

ALTER INDEX index_name ALTER EXPRESSION 0 SET STATISTICS stat_target; --
Refer expression by its number
ALTER INDEX index_name ALTER EXPRESSION (x + y) SET STATISTICS

stat_target;

-- Refer expression by its definition

Don't like either of those particularly, but what about just targeting
a column by column number, independently of whether it's an expression?

ALTER INDEX myindex ALTER COLUMN 4 SET STATISTICS 1000;

I completely agree with that.
If no objections, I will write a patch for that.

Please, find it in attached patch.

I decided to forbid referencing columns by numbers for tables, because
those numbers could contain gaps. Also, I forbid altering statistics
target for non-expression index columns, because it has no effect.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

alter-index-statistics-1.patchapplication/octet-stream; name=alter-index-statistics-1.patchDownload
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
new file mode 100644
index ad77b57..34ab710
*** a/doc/src/sgml/ref/alter_index.sgml
--- b/doc/src/sgml/ref/alter_index.sgml
*************** ALTER INDEX [ IF EXISTS ] <replaceable c
*** 26,31 ****
--- 26,33 ----
  ALTER INDEX <replaceable class="PARAMETER">name</replaceable> DEPENDS ON EXTENSION <replaceable class="PARAMETER">extension_name</replaceable>
  ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> SET ( <replaceable class="PARAMETER">storage_parameter</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )
  ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> RESET ( <replaceable class="PARAMETER">storage_parameter</replaceable> [, ... ] )
+ ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> ALTER [ COLUMN ] <replaceable class="PARAMETER">column_number</replaceable>
+     SET STATISTICS <replaceable class="PARAMETER">integer</replaceable>
  ALTER INDEX ALL IN TABLESPACE <replaceable class="PARAMETER">name</replaceable> [ OWNED BY <replaceable class="PARAMETER">role_name</replaceable> [, ... ] ]
      SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable> [ NOWAIT ]
  </synopsis>
*************** ALTER INDEX ALL IN TABLESPACE <replaceab
*** 110,115 ****
--- 112,134 ----
      </listitem>
     </varlistentry>
  
+    <varlistentry>
+     <term><literal>ALTER [ COLUMN ] <replaceable class="PARAMETER">column_number</replaceable> SET STATISTICS <replaceable class="PARAMETER">integer</replaceable></literal></term>
+     <listitem>
+      <para>
+       This form sets statistics-gathering target for an indexed expression
+       referenced by its number.  This value is used in subsequent
+       <xref linkend="sql-analyze"> operations.
+       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 (<xref linkend="guc-default-statistics-target">).
+       For more information on the use of statistics by the
+       <productname>PostgreSQL</productname> query planner, refer to
+       <xref linkend="planner-stats">.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
    </variablelist>
    </para>
  
*************** ALTER INDEX ALL IN TABLESPACE <replaceab
*** 131,136 ****
--- 150,164 ----
       </varlistentry>
  
       <varlistentry>
+       <term><replaceable class="PARAMETER">column_number</replaceable></term>
+       <listitem>
+        <para>
+         The number of index column starting from 1.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
        <term><replaceable class="PARAMETER">name</replaceable></term>
        <listitem>
         <para>
*************** ALTER INDEX distributors SET (fillfactor
*** 235,240 ****
--- 263,276 ----
  REINDEX INDEX distributors;
  </programlisting></para>
  
+   <para>
+    To set statistics-gathering target for an indexed expression:
+ <programlisting>
+ CREATE INDEX coord_idx ON measured (x, y, (z + t));
+ ALTER INDEX coord_idx ALTER COLUMN 3 SET STATISTICS 1000;
+ </programlisting>
+   </para>
+ 
   </refsect1>
  
   <refsect1>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
new file mode 100644
index 7959120..90fb5d7
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** static ObjectAddress ATExecAddIdentity(R
*** 367,375 ****
  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,
  					Node *newValue, LOCKMODE lockmode);
! static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName,
  					Node *newValue, LOCKMODE lockmode);
  static ObjectAddress ATExecSetOptions(Relation rel, const char *colName,
  				 Node *options, bool isReset, LOCKMODE lockmode);
--- 367,375 ----
  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, int16 colNum,
  					Node *newValue, LOCKMODE lockmode);
! 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);
*************** ATPrepCmd(List **wqueue, Relation rel, A
*** 3516,3522 ****
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
  			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
  			/* Performs own permission checks */
! 			ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode);
  			pass = AT_PASS_MISC;
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
--- 3516,3522 ----
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
  			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
  			/* Performs own permission checks */
! 			ATPrepSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode);
  			pass = AT_PASS_MISC;
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
*************** ATExecCmd(List **wqueue, AlteredTableInf
*** 3840,3846 ****
  			address = ATExecSetNotNull(tab, rel, cmd->name, lockmode);
  			break;
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
! 			address = ATExecSetStatistics(rel, cmd->name, cmd->def, lockmode);
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
  			address = ATExecSetOptions(rel, cmd->name, cmd->def, false, lockmode);
--- 3840,3846 ----
  			address = ATExecSetNotNull(tab, rel, cmd->name, lockmode);
  			break;
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
! 			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);
*************** ATExecDropIdentity(Relation rel, const c
*** 6095,6101 ****
   * ALTER TABLE ALTER COLUMN SET STATISTICS
   */
  static void
! ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode)
  {
  	/*
  	 * We do our own permission checking because (a) we want to allow SET
--- 6095,6101 ----
   * ALTER TABLE ALTER COLUMN SET STATISTICS
   */
  static void
! 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
*************** ATPrepSetStatistics(Relation rel, const 
*** 6113,6118 ****
--- 6113,6128 ----
  				 errmsg("\"%s\" is not a table, materialized view, index, or foreign table",
  						RelationGetRelationName(rel))));
  
+ 	/*
+ 	 * We do allow referencing columns by names only for indexes, because
+ 	 * table column numbers could contain gaps.
+ 	 */
+ 	if (rel->rd_rel->relkind != RELKIND_INDEX && !colName)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ 				 errmsg("\"%s\" is not a index, but only index columns could be refered by number",
+ 						RelationGetRelationName(rel))));
+ 
  	/* Permissions checks */
  	if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
  		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
*************** ATPrepSetStatistics(Relation rel, const 
*** 6123,6129 ****
   * Return value is the address of the modified column
   */
  static ObjectAddress
! ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode)
  {
  	int			newtarget;
  	Relation	attrelation;
--- 6133,6139 ----
   * Return value is the address of the modified column
   */
  static ObjectAddress
! ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode)
  {
  	int			newtarget;
  	Relation	attrelation;
*************** ATExecSetStatistics(Relation rel, const 
*** 6156,6168 ****
  
  	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
  
! 	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))));
  	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
  
  	attnum = attrtuple->attnum;
--- 6166,6192 ----
  
  	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
  
! 	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))));
! 	}
  
  	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
  
  	attnum = attrtuple->attnum;
*************** ATExecSetStatistics(Relation rel, const 
*** 6172,6177 ****
--- 6196,6209 ----
  				 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
new file mode 100644
index 36bf1dc..5ce2285
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyAlterTableCmd(const AlterTableCmd *
*** 3082,3087 ****
--- 3082,3088 ----
  
  	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
new file mode 100644
index 5bcf031..0b6fc7c
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalAlterTableCmd(const AlterTableCmd 
*** 1098,1103 ****
--- 1098,1104 ----
  {
  	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
new file mode 100644
index 7e03624..2a34058
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** alter_table_cmd:
*** 2109,2114 ****
--- 2109,2129 ----
  					n->def = (Node *) makeInteger($6);
  					$$ = (Node *)n;
  				}
+ 			/* ALTER TABLE <name> ALTER [COLUMN] <colnum> SET STATISTICS <SignedIconst> */
+ 			| ALTER opt_column Iconst SET STATISTICS SignedIconst
+ 				{
+ 					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)));
+ 
+ 					AlterTableCmd *n = makeNode(AlterTableCmd);
+ 					n->subtype = AT_SetStatistics;
+ 					n->num = (int16) $3;
+ 					n->def = (Node *) makeInteger($6);
+ 					$$ = (Node *)n;
+ 				}
  			/* ALTER TABLE <name> ALTER [COLUMN] <colname> 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
new file mode 100644
index e66bb03..ea41eb0
*** a/src/backend/utils/cache/syscache.c
--- b/src/backend/utils/cache/syscache.c
*************** SearchSysCacheExistsAttName(Oid relid, c
*** 1257,1262 ****
--- 1257,1308 ----
  
  
  /*
+  * 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 tuple;
+ 	newtuple = heap_copytuple(tuple);
+ 	ReleaseSysCache(tuple);
+ 	return newtuple;
+ }
+ 
+ 
+ /*
   * SysCacheGetAttr
   *
   *		Given a tuple previously fetched by SearchSysCache(),
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
new file mode 100644
index 8720e71..916ade4
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct AlterTableCmd	/* one subc
*** 1753,1758 ****
--- 1753,1760 ----
  	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
new file mode 100644
index 246601c..87967b3
*** a/src/include/utils/syscache.h
--- b/src/include/utils/syscache.h
*************** extern HeapTuple SearchSysCacheAttName(O
*** 131,136 ****
--- 131,139 ----
  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
new file mode 100644
index 8aadbb8..cee919d
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*************** SELECT * FROM tmp;
*** 94,99 ****
--- 94,114 ----
           | 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;
+ 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/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
new file mode 100644
index c41b487..b61bb01
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
*************** INSERT INTO tmp (a, b, c, d, e, f, g, h,
*** 142,147 ****
--- 142,161 ----
  
  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;
+ 
+ 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;
  
  
#5Amit Kapila
amit.kapila16@gmail.com
In reply to: Alexander Korotkov (#4)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On Thu, Jun 1, 2017 at 6:40 PM, Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

On Wed, May 31, 2017 at 7:18 PM, Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:

Don't like either of those particularly, but what about just targeting
a column by column number, independently of whether it's an expression?

ALTER INDEX myindex ALTER COLUMN 4 SET STATISTICS 1000;

I completely agree with that.
If no objections, I will write a patch for that.

Please, find it in attached patch.

I decided to forbid referencing columns by numbers for tables, because those
numbers could contain gaps. Also, I forbid altering statistics target for
non-expression index columns, because it has no effect.

In order to avoid losing track of this patch, I think it is better to
add it in open items list for 10.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Kapila (#5)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

Amit Kapila <amit.kapila16@gmail.com> writes:

In order to avoid losing track of this patch, I think it is better to
add it in open items list for 10.

This is an entirely new feature, not a bug fix, and thus certainly not an
open item for v10. Please stick it in the next commitfest, instead.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Amit Kapila
amit.kapila16@gmail.com
In reply to: Tom Lane (#6)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On Sun, Jun 4, 2017 at 10:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Amit Kapila <amit.kapila16@gmail.com> writes:

In order to avoid losing track of this patch, I think it is better to
add it in open items list for 10.

This is an entirely new feature, not a bug fix, and thus certainly not an
open item for v10. Please stick it in the next commitfest, instead.

Okay, that makes sense. Sorry, I missed the point in the first read
of the mail.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Alexander Korotkov (#4)
1 attachment(s)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On Thu, Jun 1, 2017 at 4:10 PM, Alexander Korotkov <
a.korotkov@postgrespro.ru> wrote:

On Wed, May 31, 2017 at 7:18 PM, Alexander Korotkov <
a.korotkov@postgrespro.ru> wrote:

On Wed, May 31, 2017 at 6:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:

I've discovered that PostgreSQL is able to run following kind of

queries in

order to change statistic-gathering target for an indexed expression.

ALTER INDEX index_name ALTER COLUMN expr SET STATISTICS stat_target;

It's been previously discussed in [1].

I think this should be fixed not just in docs. This is why I've

started

thread in pgsql-hackers. For me usage of internal column names "expr",
"expr1", "expr2" etc. looks weird. And I think we should replace it

with a

better syntax. What do you think about these options?

ALTER INDEX index_name ALTER EXPRESSION 0 SET STATISTICS stat_target;

--

Refer expression by its number
ALTER INDEX index_name ALTER EXPRESSION (x + y) SET STATISTICS

stat_target;

-- Refer expression by its definition

Don't like either of those particularly, but what about just targeting
a column by column number, independently of whether it's an expression?

ALTER INDEX myindex ALTER COLUMN 4 SET STATISTICS 1000;

I completely agree with that.
If no objections, I will write a patch for that.

Please, find it in attached patch.

I decided to forbid referencing columns by numbers for tables, because
those numbers could contain gaps. Also, I forbid altering statistics
target for non-expression index columns, because it has no effect.

Patch rebased to current master is attached.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

alter-index-statistics-2.patchapplication/octet-stream; name=alter-index-statistics-2.patchDownload
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
new file mode 100644
index ad77b57..34ab710
*** a/doc/src/sgml/ref/alter_index.sgml
--- b/doc/src/sgml/ref/alter_index.sgml
*************** ALTER INDEX [ IF EXISTS ] <replaceable c
*** 26,31 ****
--- 26,33 ----
  ALTER INDEX <replaceable class="PARAMETER">name</replaceable> DEPENDS ON EXTENSION <replaceable class="PARAMETER">extension_name</replaceable>
  ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> SET ( <replaceable class="PARAMETER">storage_parameter</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )
  ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> RESET ( <replaceable class="PARAMETER">storage_parameter</replaceable> [, ... ] )
+ ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> ALTER [ COLUMN ] <replaceable class="PARAMETER">column_number</replaceable>
+     SET STATISTICS <replaceable class="PARAMETER">integer</replaceable>
  ALTER INDEX ALL IN TABLESPACE <replaceable class="PARAMETER">name</replaceable> [ OWNED BY <replaceable class="PARAMETER">role_name</replaceable> [, ... ] ]
      SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable> [ NOWAIT ]
  </synopsis>
*************** ALTER INDEX ALL IN TABLESPACE <replaceab
*** 110,115 ****
--- 112,134 ----
      </listitem>
     </varlistentry>
  
+    <varlistentry>
+     <term><literal>ALTER [ COLUMN ] <replaceable class="PARAMETER">column_number</replaceable> SET STATISTICS <replaceable class="PARAMETER">integer</replaceable></literal></term>
+     <listitem>
+      <para>
+       This form sets statistics-gathering target for an indexed expression
+       referenced by its number.  This value is used in subsequent
+       <xref linkend="sql-analyze"> operations.
+       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 (<xref linkend="guc-default-statistics-target">).
+       For more information on the use of statistics by the
+       <productname>PostgreSQL</productname> query planner, refer to
+       <xref linkend="planner-stats">.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
    </variablelist>
    </para>
  
*************** ALTER INDEX ALL IN TABLESPACE <replaceab
*** 131,136 ****
--- 150,164 ----
       </varlistentry>
  
       <varlistentry>
+       <term><replaceable class="PARAMETER">column_number</replaceable></term>
+       <listitem>
+        <para>
+         The number of index column starting from 1.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
        <term><replaceable class="PARAMETER">name</replaceable></term>
        <listitem>
         <para>
*************** ALTER INDEX distributors SET (fillfactor
*** 235,240 ****
--- 263,276 ----
  REINDEX INDEX distributors;
  </programlisting></para>
  
+   <para>
+    To set statistics-gathering target for an indexed expression:
+ <programlisting>
+ CREATE INDEX coord_idx ON measured (x, y, (z + t));
+ ALTER INDEX coord_idx ALTER COLUMN 3 SET STATISTICS 1000;
+ </programlisting>
+   </para>
+ 
   </refsect1>
  
   <refsect1>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
new file mode 100644
index 0f08245..8e8b4ea
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** static ObjectAddress ATExecAddIdentity(R
*** 375,383 ****
  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,
  					Node *newValue, LOCKMODE lockmode);
! static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName,
  					Node *newValue, LOCKMODE lockmode);
  static ObjectAddress ATExecSetOptions(Relation rel, const char *colName,
  				 Node *options, bool isReset, LOCKMODE lockmode);
--- 375,383 ----
  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, int16 colNum,
  					Node *newValue, LOCKMODE lockmode);
! 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);
*************** ATPrepCmd(List **wqueue, Relation rel, A
*** 3525,3531 ****
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
  			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
  			/* Performs own permission checks */
! 			ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode);
  			pass = AT_PASS_MISC;
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
--- 3525,3531 ----
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
  			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
  			/* Performs own permission checks */
! 			ATPrepSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode);
  			pass = AT_PASS_MISC;
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
*************** ATExecCmd(List **wqueue, AlteredTableInf
*** 3848,3854 ****
  			address = ATExecSetNotNull(tab, rel, cmd->name, lockmode);
  			break;
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
! 			address = ATExecSetStatistics(rel, cmd->name, cmd->def, lockmode);
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
  			address = ATExecSetOptions(rel, cmd->name, cmd->def, false, lockmode);
--- 3848,3854 ----
  			address = ATExecSetNotNull(tab, rel, cmd->name, lockmode);
  			break;
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
! 			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);
*************** ATExecDropIdentity(Relation rel, const c
*** 6120,6126 ****
   * ALTER TABLE ALTER COLUMN SET STATISTICS
   */
  static void
! ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode)
  {
  	/*
  	 * We do our own permission checking because (a) we want to allow SET
--- 6120,6126 ----
   * ALTER TABLE ALTER COLUMN SET STATISTICS
   */
  static void
! 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
*************** ATPrepSetStatistics(Relation rel, const 
*** 6138,6143 ****
--- 6138,6153 ----
  				 errmsg("\"%s\" is not a table, materialized view, index, or foreign table",
  						RelationGetRelationName(rel))));
  
+ 	/*
+ 	 * We do allow referencing columns by names only for indexes, because
+ 	 * table column numbers could contain gaps.
+ 	 */
+ 	if (rel->rd_rel->relkind != RELKIND_INDEX && !colName)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ 				 errmsg("\"%s\" is not a index, but only index columns could be refered by number",
+ 						RelationGetRelationName(rel))));
+ 
  	/* Permissions checks */
  	if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
  		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
*************** ATPrepSetStatistics(Relation rel, const 
*** 6148,6154 ****
   * Return value is the address of the modified column
   */
  static ObjectAddress
! ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode)
  {
  	int			newtarget;
  	Relation	attrelation;
--- 6158,6164 ----
   * Return value is the address of the modified column
   */
  static ObjectAddress
! ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode)
  {
  	int			newtarget;
  	Relation	attrelation;
*************** ATExecSetStatistics(Relation rel, const 
*** 6181,6193 ****
  
  	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
  
! 	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))));
  	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
  
  	attnum = attrtuple->attnum;
--- 6191,6217 ----
  
  	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
  
! 	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))));
! 	}
  
  	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
  
  	attnum = attrtuple->attnum;
*************** ATExecSetStatistics(Relation rel, const 
*** 6197,6202 ****
--- 6221,6234 ----
  				 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
new file mode 100644
index 7204169..3f6efdd
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyAlterTableCmd(const AlterTableCmd *
*** 3085,3090 ****
--- 3085,3091 ----
  
  	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
new file mode 100644
index 8d92c03..11731da
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalAlterTableCmd(const AlterTableCmd 
*** 1098,1103 ****
--- 1098,1104 ----
  {
  	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
new file mode 100644
index 7d0de99..c353a0e
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** alter_table_cmd:
*** 2078,2083 ****
--- 2078,2098 ----
  					n->def = (Node *) makeInteger($6);
  					$$ = (Node *)n;
  				}
+ 			/* ALTER TABLE <name> ALTER [COLUMN] <colnum> SET STATISTICS <SignedIconst> */
+ 			| ALTER opt_column Iconst SET STATISTICS SignedIconst
+ 				{
+ 					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)));
+ 
+ 					AlterTableCmd *n = makeNode(AlterTableCmd);
+ 					n->subtype = AT_SetStatistics;
+ 					n->num = (int16) $3;
+ 					n->def = (Node *) makeInteger($6);
+ 					$$ = (Node *)n;
+ 				}
  			/* ALTER TABLE <name> ALTER [COLUMN] <colname> 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
new file mode 100644
index 607fe9d..d83ffda
*** a/src/backend/utils/cache/syscache.c
--- b/src/backend/utils/cache/syscache.c
*************** SearchSysCacheExistsAttName(Oid relid, c
*** 1257,1262 ****
--- 1257,1308 ----
  
  
  /*
+  * 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 tuple;
+ 	newtuple = heap_copytuple(tuple);
+ 	ReleaseSysCache(tuple);
+ 	return newtuple;
+ }
+ 
+ 
+ /*
   * SysCacheGetAttr
   *
   *		Given a tuple previously fetched by SearchSysCache(),
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
new file mode 100644
index 5f2a4a7..5bc6243
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct AlterTableCmd	/* one subc
*** 1772,1777 ****
--- 1772,1779 ----
  	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
new file mode 100644
index 8352b40..8a92ea2
*** a/src/include/utils/syscache.h
--- b/src/include/utils/syscache.h
*************** extern HeapTuple SearchSysCacheAttName(O
*** 131,136 ****
--- 131,139 ----
  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
new file mode 100644
index ed03cb9..64160a8
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*************** SELECT * FROM tmp;
*** 94,99 ****
--- 94,114 ----
           | 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;
+ 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/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
new file mode 100644
index 9a20dd1..57f44c4
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
*************** INSERT INTO tmp (a, b, c, d, e, f, g, h,
*** 142,147 ****
--- 142,161 ----
  
  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;
+ 
+ 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;
  
  
#9Adrien Nayrat
adrien.nayrat@dalibo.com
In reply to: Alexander Korotkov (#8)
1 attachment(s)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On 08/30/2017 02:26 PM, Alexander Korotkov wrote:

Patch rebased to current master is attached.

Hello,

I reviewed this patch. It works as expected but I have few remarks :

* There is a warning during compilation :

gram.y: In function ‘base_yyparse’:
gram.y:2090:6: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
AlterTableCmd *n = makeNode(AlterTableCmd);
^~~~~~~~~~~~~

If I understand we should declare AlterTableCmd *n [...] before "if"?

* I propose to add "Stats target" information in psql verbose output command
\d+ (patch attached with test)

\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"

* psql completion is missing (added to previous patch)

Regards,

--
Adrien NAYRAT

http://dalibo.com - http://dalibo.org

Attachments:

alter_index.patchtext/x-patch; name=alter_index.patchDownload
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 1583cfa998..baa739a8c0 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 <name> */
 	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 <name> ALTER COLUMN <colnum> */
+	else if (Matches6("ALTER", "INDEX", MatchAny, "ALTER", "COLUMN", MatchAny))
+		COMPLETE_WITH_CONST("SET STATISTICS");
 	/* ALTER INDEX <name> SET */
 	else if (Matches4("ALTER", "INDEX", MatchAny, "SET"))
 		COMPLETE_WITH_LIST2("(", "TABLESPACE");
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 64160a8b4d..0f36423163 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -103,6 +103,15 @@ 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.
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 57f44c445d..e6f6669880 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -150,6 +150,8 @@ 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;
#10Alexander Korotkov
a.korotkov@postgrespro.ru
In reply to: Adrien Nayrat (#9)
1 attachment(s)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

Hi, Adrien!

On Mon, Sep 4, 2017 at 3:57 PM, Adrien Nayrat <adrien.nayrat@dalibo.com>
wrote:

On 08/30/2017 02:26 PM, Alexander Korotkov wrote:
I reviewed this patch. It works as expected but I have few remarks :

Thank you for reviewing my patch!.

* There is a warning during compilation :

gram.y: In function ‘base_yyparse’:
gram.y:2090:6: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
AlterTableCmd *n = makeNode(AlterTableCmd);
^~~~~~~~~~~~~

Fixed.

If I understand we should declare AlterTableCmd *n [...] before "if"?

* I propose to add "Stats target" information in psql verbose output
command
\d+ (patch attached with test)

\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"

* psql completion is missing (added to previous patch)

Looks good for me. I've integrated those changes in the patch.
New revision is attached.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

alter-index-statistics-3.patchapplication/octet-stream; name=alter-index-statistics-3.patchDownload
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
new file mode 100644
index ad77b57..34ab710
*** a/doc/src/sgml/ref/alter_index.sgml
--- b/doc/src/sgml/ref/alter_index.sgml
*************** ALTER INDEX [ IF EXISTS ] <replaceable c
*** 26,31 ****
--- 26,33 ----
  ALTER INDEX <replaceable class="PARAMETER">name</replaceable> DEPENDS ON EXTENSION <replaceable class="PARAMETER">extension_name</replaceable>
  ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> SET ( <replaceable class="PARAMETER">storage_parameter</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )
  ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> RESET ( <replaceable class="PARAMETER">storage_parameter</replaceable> [, ... ] )
+ ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> ALTER [ COLUMN ] <replaceable class="PARAMETER">column_number</replaceable>
+     SET STATISTICS <replaceable class="PARAMETER">integer</replaceable>
  ALTER INDEX ALL IN TABLESPACE <replaceable class="PARAMETER">name</replaceable> [ OWNED BY <replaceable class="PARAMETER">role_name</replaceable> [, ... ] ]
      SET TABLESPACE <replaceable class="PARAMETER">new_tablespace</replaceable> [ NOWAIT ]
  </synopsis>
*************** ALTER INDEX ALL IN TABLESPACE <replaceab
*** 110,115 ****
--- 112,134 ----
      </listitem>
     </varlistentry>
  
+    <varlistentry>
+     <term><literal>ALTER [ COLUMN ] <replaceable class="PARAMETER">column_number</replaceable> SET STATISTICS <replaceable class="PARAMETER">integer</replaceable></literal></term>
+     <listitem>
+      <para>
+       This form sets statistics-gathering target for an indexed expression
+       referenced by its number.  This value is used in subsequent
+       <xref linkend="sql-analyze"> operations.
+       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 (<xref linkend="guc-default-statistics-target">).
+       For more information on the use of statistics by the
+       <productname>PostgreSQL</productname> query planner, refer to
+       <xref linkend="planner-stats">.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
    </variablelist>
    </para>
  
*************** ALTER INDEX ALL IN TABLESPACE <replaceab
*** 131,136 ****
--- 150,164 ----
       </varlistentry>
  
       <varlistentry>
+       <term><replaceable class="PARAMETER">column_number</replaceable></term>
+       <listitem>
+        <para>
+         The number of index column starting from 1.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
        <term><replaceable class="PARAMETER">name</replaceable></term>
        <listitem>
         <para>
*************** ALTER INDEX distributors SET (fillfactor
*** 235,240 ****
--- 263,276 ----
  REINDEX INDEX distributors;
  </programlisting></para>
  
+   <para>
+    To set statistics-gathering target for an indexed expression:
+ <programlisting>
+ CREATE INDEX coord_idx ON measured (x, y, (z + t));
+ ALTER INDEX coord_idx ALTER COLUMN 3 SET STATISTICS 1000;
+ </programlisting>
+   </para>
+ 
   </refsect1>
  
   <refsect1>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
new file mode 100644
index 0f08245..8e8b4ea
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** static ObjectAddress ATExecAddIdentity(R
*** 375,383 ****
  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,
  					Node *newValue, LOCKMODE lockmode);
! static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName,
  					Node *newValue, LOCKMODE lockmode);
  static ObjectAddress ATExecSetOptions(Relation rel, const char *colName,
  				 Node *options, bool isReset, LOCKMODE lockmode);
--- 375,383 ----
  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, int16 colNum,
  					Node *newValue, LOCKMODE lockmode);
! 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);
*************** ATPrepCmd(List **wqueue, Relation rel, A
*** 3525,3531 ****
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
  			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
  			/* Performs own permission checks */
! 			ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode);
  			pass = AT_PASS_MISC;
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
--- 3525,3531 ----
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
  			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
  			/* Performs own permission checks */
! 			ATPrepSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode);
  			pass = AT_PASS_MISC;
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
*************** ATExecCmd(List **wqueue, AlteredTableInf
*** 3848,3854 ****
  			address = ATExecSetNotNull(tab, rel, cmd->name, lockmode);
  			break;
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
! 			address = ATExecSetStatistics(rel, cmd->name, cmd->def, lockmode);
  			break;
  		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
  			address = ATExecSetOptions(rel, cmd->name, cmd->def, false, lockmode);
--- 3848,3854 ----
  			address = ATExecSetNotNull(tab, rel, cmd->name, lockmode);
  			break;
  		case AT_SetStatistics:	/* ALTER COLUMN SET STATISTICS */
! 			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);
*************** ATExecDropIdentity(Relation rel, const c
*** 6120,6126 ****
   * ALTER TABLE ALTER COLUMN SET STATISTICS
   */
  static void
! ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode)
  {
  	/*
  	 * We do our own permission checking because (a) we want to allow SET
--- 6120,6126 ----
   * ALTER TABLE ALTER COLUMN SET STATISTICS
   */
  static void
! 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
*************** ATPrepSetStatistics(Relation rel, const 
*** 6138,6143 ****
--- 6138,6153 ----
  				 errmsg("\"%s\" is not a table, materialized view, index, or foreign table",
  						RelationGetRelationName(rel))));
  
+ 	/*
+ 	 * We do allow referencing columns by names only for indexes, because
+ 	 * table column numbers could contain gaps.
+ 	 */
+ 	if (rel->rd_rel->relkind != RELKIND_INDEX && !colName)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ 				 errmsg("\"%s\" is not a index, but only index columns could be refered by number",
+ 						RelationGetRelationName(rel))));
+ 
  	/* Permissions checks */
  	if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
  		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
*************** ATPrepSetStatistics(Relation rel, const 
*** 6148,6154 ****
   * Return value is the address of the modified column
   */
  static ObjectAddress
! ATExecSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode)
  {
  	int			newtarget;
  	Relation	attrelation;
--- 6158,6164 ----
   * Return value is the address of the modified column
   */
  static ObjectAddress
! ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode)
  {
  	int			newtarget;
  	Relation	attrelation;
*************** ATExecSetStatistics(Relation rel, const 
*** 6181,6193 ****
  
  	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
  
! 	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))));
  	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
  
  	attnum = attrtuple->attnum;
--- 6191,6217 ----
  
  	attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
  
! 	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))));
! 	}
  
  	attrtuple = (Form_pg_attribute) GETSTRUCT(tuple);
  
  	attnum = attrtuple->attnum;
*************** ATExecSetStatistics(Relation rel, const 
*** 6197,6202 ****
--- 6221,6234 ----
  				 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
new file mode 100644
index f9ddf4e..9bae264
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyAlterTableCmd(const AlterTableCmd *
*** 3087,3092 ****
--- 3087,3093 ----
  
  	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
new file mode 100644
index 8d92c03..11731da
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalAlterTableCmd(const AlterTableCmd 
*** 1098,1103 ****
--- 1098,1104 ----
  {
  	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
new file mode 100644
index 7d0de99..5eb3981
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** alter_table_cmd:
*** 2078,2083 ****
--- 2078,2099 ----
  					n->def = (Node *) makeInteger($6);
  					$$ = (Node *)n;
  				}
+ 			/* ALTER TABLE <name> ALTER [COLUMN] <colnum> SET STATISTICS <SignedIconst> */
+ 			| 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 <name> ALTER [COLUMN] <colname> 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
new file mode 100644
index 607fe9d..d83ffda
*** a/src/backend/utils/cache/syscache.c
--- b/src/backend/utils/cache/syscache.c
*************** SearchSysCacheExistsAttName(Oid relid, c
*** 1257,1262 ****
--- 1257,1308 ----
  
  
  /*
+  * 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 tuple;
+ 	newtuple = heap_copytuple(tuple);
+ 	ReleaseSysCache(tuple);
+ 	return newtuple;
+ }
+ 
+ 
+ /*
   * SysCacheGetAttr
   *
   *		Given a tuple previously fetched by SearchSysCache(),
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index f6049cc..6fb9bdd
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeOneTableDetails(const char *sche
*** 1742,1747 ****
--- 1742,1748 ----
  	{
  		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)
*************** describeOneTableDetails(const char *sche
*** 1841,1846 ****
--- 1842,1848 ----
  
  			/* 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
new file mode 100644
index 1583cfa..baa739a
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(const char *text, int st
*** 1644,1650 ****
  								   "UNION SELECT 'ALL IN TABLESPACE'");
  	/* ALTER INDEX <name> */
  	else if (Matches3("ALTER", "INDEX", MatchAny))
! 		COMPLETE_WITH_LIST4("OWNER TO", "RENAME TO", "SET", "RESET");
  	/* ALTER INDEX <name> SET */
  	else if (Matches4("ALTER", "INDEX", MatchAny, "SET"))
  		COMPLETE_WITH_LIST2("(", "TABLESPACE");
--- 1644,1653 ----
  								   "UNION SELECT 'ALL IN TABLESPACE'");
  	/* ALTER INDEX <name> */
  	else if (Matches3("ALTER", "INDEX", MatchAny))
! 		COMPLETE_WITH_LIST5("ALTER COLUMN", "OWNER TO", "RENAME TO", "SET", "RESET");
! 	/* ALTER INDEX <name> ALTER COLUMN <colnum> */
! 	else if (Matches6("ALTER", "INDEX", MatchAny, "ALTER", "COLUMN", MatchAny))
! 		COMPLETE_WITH_CONST("SET STATISTICS");
  	/* ALTER INDEX <name> 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
new file mode 100644
index 5f2a4a7..5bc6243
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct AlterTableCmd	/* one subc
*** 1772,1777 ****
--- 1772,1779 ----
  	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
new file mode 100644
index 8352b40..8a92ea2
*** a/src/include/utils/syscache.h
--- b/src/include/utils/syscache.h
*************** extern HeapTuple SearchSysCacheAttName(O
*** 131,136 ****
--- 131,139 ----
  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
new file mode 100644
index ed03cb9..0f36423
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*************** SELECT * FROM tmp;
*** 94,99 ****
--- 94,123 ----
           | 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
new file mode 100644
index 064adb4..8450f24
*** a/src/test/regress/expected/create_index.out
--- b/src/test/regress/expected/create_index.out
*************** DROP TABLE array_gin_test;
*** 2324,2333 ****
  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
  gin, for table "public.array_index_op_test"
  Options: fastupdate=on, gin_pending_list_limit=128
  
--- 2324,2333 ----
  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 | 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
new file mode 100644
index 9a20dd1..e6f6669
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
*************** INSERT INTO tmp (a, b, c, d, e, f, g, h,
*** 142,147 ****
--- 142,163 ----
  
  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;
  
  
#11Adrien Nayrat
adrien.nayrat@dalibo.com
In reply to: Alexander Korotkov (#10)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On 09/04/2017 06:16 PM, Alexander Korotkov wrote:

Looks good for me.  I've integrated those changes in the patch.
New revision is attached.

Thanks, I changed status to "ready for commiter".

--
Adrien NAYRAT

http://dalibo.com - http://dalibo.org

#12Simon Riggs
simon@2ndquadrant.com
In reply to: Adrien Nayrat (#11)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On 4 September 2017 at 10:30, Adrien Nayrat <adrien.nayrat@dalibo.com> wrote:

On 09/04/2017 06:16 PM, Alexander Korotkov wrote:

Looks good for me. I've integrated those changes in the patch.
New revision is attached.

Thanks, I changed status to "ready for commiter".

This looks useful and good to me.

I've changed this line of code to return NULL rather than return tuple
if (!HeapTupleIsValid(tuple))
return tuple;

Other than that, I'm good to commit.

Any last minute objections?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#12)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

Simon Riggs <simon@2ndquadrant.com> writes:

Other than that, I'm good to commit.
Any last minute objections?

The docs and comments could stand a bit closer copy-editing by a
native English speaker. Other than that, it seemed sane in a
quick read-through.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Simon Riggs
simon@2ndquadrant.com
In reply to: Tom Lane (#13)
Re: ALTER INDEX .. SET STATISTICS ... behaviour

On 6 September 2017 at 10:27, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@2ndquadrant.com> writes:

Other than that, I'm good to commit.
Any last minute objections?

The docs and comments could stand a bit closer copy-editing by a
native English speaker. Other than that, it seemed sane in a
quick read-through.

Will do

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers