Thread: Hash index creation warning
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? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Attachment
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.
David G Johnston <david.g.johnston@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
On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: > David G Johnston <david.g.johnston@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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Attachment
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.johnston@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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On 18 October 2014 at 15:36, Bruce Momjian <bruce@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.johnston@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
On 6/12/15 5:00 PM, Thom Brown wrote: > On 18 October 2014 at 15:36, Bruce Momjian <bruce@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.johnston@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
On Tue, Jun 23, 2015 at 9:06 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote: > On 6/12/15 5:00 PM, Thom Brown wrote: >> >> On 18 October 2014 at 15:36, Bruce Momjian <bruce@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.johnston@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
Attachment
On Mon, Jun 22, 2015 at 8:46 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Tue, Jun 23, 2015 at 9:06 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote: >> On 6/12/15 5:00 PM, Thom Brown wrote: >>> >>> On 18 October 2014 at 15:36, Bruce Momjian <bruce@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.johnston@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
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
Attachment
On 18 October 2014 at 02:36, Bruce Momjian <bruce@momjian.us> wrote: > On Fri, Oct 17, 2014 at 12:56:52PM -0400, Tom Lane wrote: >> David G Johnston <david.g.johnston@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. While I don't think it should explain all the details, "WAL-logged" will mean *nothing* to most users, including most of those who're using streaming replication, PITR, etc. I would strongly prefer to see something that conveys some meaning to a user who doesn't know PostgreSQL's innards, since by the time "WAL logged" means much to you, you've got a good chance of having already learned that hash indexes aren't crash-safe. Or of reading the manual. Perhaps: WARNING: hash indexes are not crash-safe, not replicated, and their use is discouraged ? -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Jun 24, 2015 at 1:45 AM, Craig Ringer <craig@2ndquadrant.com> wrote: > WARNING: hash indexes are not crash-safe, not replicated, and their > use is discouraged +1 -- Peter Geoghegan
On Wed, Jun 24, 2015 at 3:06 AM, Michael Paquier <michael.paquier@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
On Wed, Jun 24, 2015 at 4:53 AM, Peter Geoghegan <pg@heroku.com> wrote: > On Wed, Jun 24, 2015 at 1:45 AM, Craig Ringer <craig@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
On Fri, Jun 26, 2015 at 11:40:27AM -0400, Robert Haas wrote: > On Wed, Jun 24, 2015 at 4:53 AM, Peter Geoghegan <pg@heroku.com> wrote: > > On Wed, Jun 24, 2015 at 1:45 AM, Craig Ringer <craig@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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +