Re: [HACKERS] Hash index creation warning
On Fri, Jun 26, 2015 at 11:40:27AM -0400, Robert Haas wrote: On Wed, Jun 24, 2015 at 4:53 AM, Peter Geoghegan p...@heroku.com wrote: On Wed, Jun 24, 2015 at 1:45 AM, Craig Ringer cr...@2ndquadrant.com wrote: WARNING: hash indexes are not crash-safe, not replicated, and their use is discouraged +1 I'm not wild about this rewording; I think that if users don't know what WAL is, they probably need to know that in order to make good decisions about whether to use hash indexes. But I don't feel super-strongly about it. Coming late to this, but I think Robert is right. WAL is used for crash recovery, PITR, and streaming replication, and I am not sure we want to specify all of those in the warning message. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On Wed, Jun 24, 2015 at 4:53 AM, Peter Geoghegan p...@heroku.com wrote: On Wed, Jun 24, 2015 at 1:45 AM, Craig Ringer cr...@2ndquadrant.com wrote: WARNING: hash indexes are not crash-safe, not replicated, and their use is discouraged +1 I'm not wild about this rewording; I think that if users don't know what WAL is, they probably need to know that in order to make good decisions about whether to use hash indexes. But I don't feel super-strongly about it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On Wed, Jun 24, 2015 at 3:06 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Jun 24, 2015 at 12:27 AM, Robert Haas wrote: I think you should be testing RelationNeedsWAL(), not the relpersistence directly. The same point applies for temporary indexes. Indeed. Patch updated attached. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On Wed, Jun 24, 2015 at 12:27 AM, Robert Haas wrote: I think you should be testing RelationNeedsWAL(), not the relpersistence directly. The same point applies for temporary indexes. Indeed. Patch updated attached. -- Michael diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 7340a1f..b450bcf 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -490,7 +490,8 @@ DefineIndex(Oid relationId, accessMethodId = HeapTupleGetOid(tuple); accessMethodForm = (Form_pg_am) GETSTRUCT(tuple); - if (strcmp(accessMethodName, hash) == 0) + if (strcmp(accessMethodName, hash) == 0 + RelationNeedsWAL(rel)) ereport(WARNING, (errmsg(hash indexes are not WAL-logged and their use is discouraged))); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 5c2e67d..b72e65d 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2342,6 +2342,9 @@ CREATE INDEX hash_txt_index ON hash_txt_heap USING hash (random text_ops); WARNING: hash indexes are not WAL-logged and their use is discouraged CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops); WARNING: hash indexes are not WAL-logged and their use is discouraged +CREATE UNLOGGED TABLE unlogged_hash_table (id int4); +CREATE INDEX unlogged_hash_index ON unlogged_hash_table USING hash (id int4_ops); +DROP TABLE unlogged_hash_table; -- CREATE INDEX hash_ovfl_index ON hash_ovfl_heap USING hash (x int4_ops); -- -- Test functional index diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index 67dd2f0..ff86953 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -684,6 +684,10 @@ CREATE INDEX hash_txt_index ON hash_txt_heap USING hash (random text_ops); CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops); +CREATE UNLOGGED TABLE unlogged_hash_table (id int4); +CREATE INDEX unlogged_hash_index ON unlogged_hash_table USING hash (id int4_ops); +DROP TABLE unlogged_hash_table; + -- CREATE INDEX hash_ovfl_index ON hash_ovfl_heap USING hash (x int4_ops); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On Mon, Jun 22, 2015 at 8:46 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Jun 23, 2015 at 9:06 AM, Jim Nasby jim.na...@bluetreble.com wrote: On 6/12/15 5:00 PM, Thom Brown wrote: On 18 October 2014 at 15:36, Bruce Momjian br...@momjian.us wrote: On Fri, Oct 17, 2014 at 02:36:55PM -0400, Bruce Momjian wrote: On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: David G Johnston david.g.johns...@gmail.com writes: The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged +1. The warning message is not the place to be trying to explain all the details. OK, updated patch attached. Patch applied. I only just noticed this item when I read the release notes. Should we bother warning when used on an unlogged table? Not really; but I think the bigger question at this point is if we want to change it this late in the game. Changing it even during beta looks acceptable to me. I think that it is mainly a matter to have a patch (here is one), and someone to push it as everybody here seem to agree that for UNLOGGED tables this warning has little sense. I think you should be testing RelationNeedsWAL(), not the relpersistence directly. The same point applies for temporary indexes. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On Tue, Jun 23, 2015 at 9:06 AM, Jim Nasby jim.na...@bluetreble.com wrote: On 6/12/15 5:00 PM, Thom Brown wrote: On 18 October 2014 at 15:36, Bruce Momjian br...@momjian.us wrote: On Fri, Oct 17, 2014 at 02:36:55PM -0400, Bruce Momjian wrote: On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: David G Johnston david.g.johns...@gmail.com writes: The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged +1. The warning message is not the place to be trying to explain all the details. OK, updated patch attached. Patch applied. I only just noticed this item when I read the release notes. Should we bother warning when used on an unlogged table? Not really; but I think the bigger question at this point is if we want to change it this late in the game. Changing it even during beta looks acceptable to me. I think that it is mainly a matter to have a patch (here is one), and someone to push it as everybody here seem to agree that for UNLOGGED tables this warning has little sense. -- Michael diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 7340a1f..49ad9d6 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -490,7 +490,8 @@ DefineIndex(Oid relationId, accessMethodId = HeapTupleGetOid(tuple); accessMethodForm = (Form_pg_am) GETSTRUCT(tuple); - if (strcmp(accessMethodName, hash) == 0) + if (strcmp(accessMethodName, hash) == 0 + rel-rd_rel-relpersistence != RELPERSISTENCE_UNLOGGED) ereport(WARNING, (errmsg(hash indexes are not WAL-logged and their use is discouraged))); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 5c2e67d..b72e65d 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2342,6 +2342,9 @@ CREATE INDEX hash_txt_index ON hash_txt_heap USING hash (random text_ops); WARNING: hash indexes are not WAL-logged and their use is discouraged CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops); WARNING: hash indexes are not WAL-logged and their use is discouraged +CREATE UNLOGGED TABLE unlogged_hash_table (id int4); +CREATE INDEX unlogged_hash_index ON unlogged_hash_table USING hash (id int4_ops); +DROP TABLE unlogged_hash_table; -- CREATE INDEX hash_ovfl_index ON hash_ovfl_heap USING hash (x int4_ops); -- -- Test functional index diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index 67dd2f0..ff86953 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -684,6 +684,10 @@ CREATE INDEX hash_txt_index ON hash_txt_heap USING hash (random text_ops); CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops); +CREATE UNLOGGED TABLE unlogged_hash_table (id int4); +CREATE INDEX unlogged_hash_index ON unlogged_hash_table USING hash (id int4_ops); +DROP TABLE unlogged_hash_table; + -- CREATE INDEX hash_ovfl_index ON hash_ovfl_heap USING hash (x int4_ops); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On 6/12/15 5:00 PM, Thom Brown wrote: On 18 October 2014 at 15:36, Bruce Momjian br...@momjian.us wrote: On Fri, Oct 17, 2014 at 02:36:55PM -0400, Bruce Momjian wrote: On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: David G Johnston david.g.johns...@gmail.com writes: The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged +1. The warning message is not the place to be trying to explain all the details. OK, updated patch attached. Patch applied. I only just noticed this item when I read the release notes. Should we bother warning when used on an unlogged table? Not really; but I think the bigger question at this point is if we want to change it this late in the game. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On 18 October 2014 at 15:36, Bruce Momjian br...@momjian.us wrote: On Fri, Oct 17, 2014 at 02:36:55PM -0400, Bruce Momjian wrote: On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: David G Johnston david.g.johns...@gmail.com writes: The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged +1. The warning message is not the place to be trying to explain all the details. OK, updated patch attached. Patch applied. I only just noticed this item when I read the release notes. Should we bother warning when used on an unlogged table? -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
On Fri, Oct 17, 2014 at 02:36:55PM -0400, Bruce Momjian wrote: On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: David G Johnston david.g.johns...@gmail.com writes: The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged +1. The warning message is not the place to be trying to explain all the details. OK, updated patch attached. Patch applied. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
Bruce Momjian wrote Now that we have the create hash index warning in 9.5, I realized that we don't warn about hash indexes with PITR, only crash recovery and streaming. This patch fixes that. Is the wording cannot be used too vague. The CREATE INDEX manual page has the words give wrong answers to queries, which might be better, but is kind of long for an error message. Suggestions? Something like the following is more specific without being more wordy: hash indexes are not WAL-logged: they are corrupted during recovery and changes do not replicate to standby servers. The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Hash-index-creation-warning-tp5823443p5823445.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash index creation warning
David G Johnston david.g.johns...@gmail.com writes: The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged +1. The warning message is not the place to be trying to explain all the details. 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
Re: [HACKERS] Hash index creation warning
On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: David G Johnston david.g.johns...@gmail.com writes: The question is whether we explain the implications of not being WAL-logged in an error message or simply state the fact and let the documentation explain the hazards - basically just output: hash indexes are not WAL-logged and their use is discouraged +1. The warning message is not the place to be trying to explain all the details. OK, updated patch attached. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml new file mode 100644 index e469b17..43df32f *** a/doc/src/sgml/ref/create_index.sgml --- b/doc/src/sgml/ref/create_index.sgml *** Indexes: *** 474,480 Also, changes to hash indexes are not replicated over streaming or file-based replication after the initial base backup, so they give wrong answers to queries that subsequently use them. ! For these reasons, hash index use is presently discouraged. /para /caution --- 474,481 Also, changes to hash indexes are not replicated over streaming or file-based replication after the initial base backup, so they give wrong answers to queries that subsequently use them. ! Hash indexes are also not properly restored during point-in-time ! recovery. For these reasons, hash index use is presently discouraged. /para /caution diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c new file mode 100644 index 8a1cb4b..3c1e90e *** a/src/backend/commands/indexcmds.c --- b/src/backend/commands/indexcmds.c *** DefineIndex(Oid relationId, *** 491,497 if (strcmp(accessMethodName, hash) == 0) ereport(WARNING, ! (errmsg(hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers))); if (stmt-unique !accessMethodForm-amcanunique) ereport(ERROR, --- 491,497 if (strcmp(accessMethodName, hash) == 0) ereport(WARNING, ! (errmsg(hash indexes are not WAL-logged and their use is discouraged))); if (stmt-unique !accessMethodForm-amcanunique) ereport(ERROR, diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out new file mode 100644 index a2bef7a..8326e94 *** a/src/test/regress/expected/create_index.out --- b/src/test/regress/expected/create_index.out *** DROP TABLE array_gin_test; *** 2238,2250 -- HASH -- CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops); ! WARNING: hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers CREATE INDEX hash_name_index ON hash_name_heap USING hash (random name_ops); ! WARNING: hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers CREATE INDEX hash_txt_index ON hash_txt_heap USING hash (random text_ops); ! WARNING: hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops); ! WARNING: hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers -- CREATE INDEX hash_ovfl_index ON hash_ovfl_heap USING hash (x int4_ops); -- -- Test functional index --- 2238,2250 -- HASH -- CREATE INDEX hash_i4_index ON hash_i4_heap USING hash (random int4_ops); ! WARNING: hash indexes are not WAL-logged and their use is discouraged CREATE INDEX hash_name_index ON hash_name_heap USING hash (random name_ops); ! WARNING: hash indexes are not WAL-logged and their use is discouraged CREATE INDEX hash_txt_index ON hash_txt_heap USING hash (random text_ops); ! WARNING: hash indexes are not WAL-logged and their use is discouraged CREATE INDEX hash_f8_index ON hash_f8_heap USING hash (random float8_ops); ! WARNING: hash indexes are not WAL-logged and their use is discouraged -- CREATE INDEX hash_ovfl_index ON hash_ovfl_heap USING hash (x int4_ops); -- -- Test functional index diff --git a/src/test/regress/expected/enum.out b/src/test/regress/expected/enum.out new file mode 100644 index fa23b52..1a61a5b *** a/src/test/regress/expected/enum.out --- b/src/test/regress/expected/enum.out *** DROP INDEX enumtest_btree; *** 383,389 -- Hash index / opclass with the = operator -- CREATE INDEX enumtest_hash ON enumtest USING hash (col); ! WARNING: hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers SELECT * FROM enumtest WHERE col = 'orange'; col --- 383,389 -- Hash index / opclass with the = operator -- CREATE INDEX enumtest_hash ON enumtest USING hash (col); ! WARNING: hash indexes are
