Re: [HACKERS] proposal: rounding up time value less than its unit.
On 9/26/14, 3:22 PM, Tom Lane wrote: We could alternatively try to split up these cases into multiple GUCs, which I guess is what you're imagining as a multi-year battle. No, I was just pointing out that even the cleanest major refactoring possible here is going to result in broken config files complaints for years. And no matter how well that goes, a second rewrite in the next major version, addressing whatever things nobody saw coming in the first redesign, is a very real possibility. The minute the GUC box is opened that far, it will not close up again in a release, or likely even two, and the griping from the field is going to take 3+ years to settle. I have no desire to waste time on partial measures either though. I think I've been clear that's my theme on this--if we're gonna mess with this significantly, let's do it right and in a big way. If there's a really trivial fix to apply, that's fine too. Throw an error when the value is between the special one and the useful minimum, no other changes; that I could see slipping into a single release without much pain. Might even be possible to write as a useful sub-step toward a bigger plan too. Wouldn't bother breaking that out as a goal unless it just happened to fall out of the larger design though. The rest of the rounding and error handling ideas wandering around seem in this uncomfortable middle ground to me. They are neither large enough to feel like a major improvement happened, nor trivial enough to apply with minimal work/risk. And this is not a bug that must be fixed ASAP--it's an unfriendly UI surprise. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] proposal: rounding up time value less than its unit.
* Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 25, 2014 at 4:49 PM, Stephen Frost sfr...@snowman.net wrote: * Gregory Smith (gregsmithpg...@gmail.com) wrote: On 9/25/14, 2:02 PM, Peter Eisentraut wrote: But having the same parameter setting mean different things in different versions is the path to complete madness. Could we go so far as to remove support for unitless time settings eventually? The fact that people are setting raw numbers in the configuration file and have to know the unit to understand what they just did has never been something I like. I could certainly get behind that idea... Tho I do understand that people will complain about backwards compatibility, etc, etc. There's also the fact that it doesn't fix the originally complained-of problem. It does fix a problem with one of the fixes proposed for that original problem, though. That's certainly an excellent point- this is really orthogonal to the actual issue of setting a value smaller than a single unit for that setting. Even if you have units attached to every GUC, an individual could take a setting which is understaood at the '1 minute' level and attempt to set it to '30 seconds', for example. On the other hand, if we're making it an error to set values without units, I'd again be behind the idea of throwing an error on the smaller-than-one-unit case- people are going to have to go update their configurations to deal with the errors from the lack-of-units, this would just be another item to fix during that process. It'd certainly be worse to change to require units and then wait anothe release to fix or change things to address the original complaint. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Thu, Sep 25, 2014 at 4:49 PM, Stephen Frost sfr...@snowman.net wrote: * Gregory Smith (gregsmithpg...@gmail.com) wrote: On 9/25/14, 2:02 PM, Peter Eisentraut wrote: But having the same parameter setting mean different things in different versions is the path to complete madness. Could we go so far as to remove support for unitless time settings eventually? The fact that people are setting raw numbers in the configuration file and have to know the unit to understand what they just did has never been something I like. I could certainly get behind that idea... Tho I do understand that people will complain about backwards compatibility, etc, etc. There's also the fact that it doesn't fix the originally complained-of problem. It does fix a problem with one of the fixes proposed for that original problem, though. -- 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] proposal: rounding up time value less than its unit.
On Fri, Sep 26, 2014 at 10:58 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 25, 2014 at 4:49 PM, Stephen Frost sfr...@snowman.net wrote: * Gregory Smith (gregsmithpg...@gmail.com) wrote: On 9/25/14, 2:02 PM, Peter Eisentraut wrote: But having the same parameter setting mean different things in different versions is the path to complete madness. Could we go so far as to remove support for unitless time settings eventually? The fact that people are setting raw numbers in the configuration file and have to know the unit to understand what they just did has never been something I like. I could certainly get behind that idea... Tho I do understand that people will complain about backwards compatibility, etc, etc. There's also the fact that it doesn't fix the originally complained-of problem. It does fix a problem with one of the fixes proposed for that original problem, though. That's certainly an excellent point- this is really orthogonal to the actual issue of setting a value smaller than a single unit for that setting. Even if you have units attached to every GUC, an individual could take a setting which is understaood at the '1 minute' level and attempt to set it to '30 seconds', for example. On the other hand, if we're making it an error to set values without units, I'd again be behind the idea of throwing an error on the smaller-than-one-unit case- people are going to have to go update their configurations to deal with the errors from the lack-of-units, this would just be another item to fix during that process. It'd certainly be worse to change to require units and then wait anothe release to fix or change things to address the original complaint. Yep, I'll buy that. If we want the narrowest possible fix for this, I think it's complain if a non-zero value would round to zero. That fixes the original complaint and changes absolutely nothing else. But I think that's kind of wussy. Yeah, rounding 29 seconds down to a special magic value of 0 is more surprising than rounding 30 seconds up to a minute, but the latter is still surprising. We're generally not averse to tighter validation, so why here? We're not talking about preventing the use of 60s to mean 1min, just the use of 42s to mean 1min, which is no different than not letting 2/29/93 mean 3/1/93, a decision about which we have no compunctions. -- 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] proposal: rounding up time value less than its unit.
Robert Haas robertmh...@gmail.com writes: If we want the narrowest possible fix for this, I think it's complain if a non-zero value would round to zero. That fixes the original complaint and changes absolutely nothing else. But I think that's kind of wussy. Yeah, rounding 29 seconds down to a special magic value of 0 is more surprising than rounding 30 seconds up to a minute, but the latter is still surprising. We're generally not averse to tighter validation, so why here? So in other words, if I set shared_buffers = 100KB, you are proposing that that be rejected because it's not an exact multiple of 8KB? This seems like it's throwing away one of the fundamental reasons why we invented GUC units in the first place. I apparently have got to make this point one more time: if the user cares about the difference between 30sec and 1min, then we erred in designing the GUC in question; it should have had a smaller unit. I am completely not impressed by arguments based on such cases. The right fix for such a case is to choose a different unit for the GUC. 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] proposal: rounding up time value less than its unit.
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haas robertmh...@gmail.com writes: If we want the narrowest possible fix for this, I think it's complain if a non-zero value would round to zero. That fixes the original complaint and changes absolutely nothing else. But I think that's kind of wussy. Yeah, rounding 29 seconds down to a special magic value of 0 is more surprising than rounding 30 seconds up to a minute, but the latter is still surprising. We're generally not averse to tighter validation, so why here? So in other words, if I set shared_buffers = 100KB, you are proposing that that be rejected because it's not an exact multiple of 8KB? This seems like it's throwing away one of the fundamental reasons why we invented GUC units in the first place. I don't believe that's what was suggested at all (it's not what I was suggesting, in any case), but rather shared_buffers = 1KB would error (erm, won't it error *anyway*? so this wouldn't be a change there..) shared_buffers = 100KB would be round the same as today. I apparently have got to make this point one more time: if the user cares about the difference between 30sec and 1min, then we erred in designing the GUC in question; it should have had a smaller unit. I am completely not impressed by arguments based on such cases. The right fix for such a case is to choose a different unit for the GUC. I don't think anyone is argueing that we should do away with the rounding rules entirely, only that we should a) require units to be specified, and b) error if the value specified is below '1 unit', but still non-zero, as it would then be rounded to zero. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 09/26/2014 10:27 AM, Stephen Frost wrote: I don't think anyone is argueing that we should do away with the rounding rules entirely, only that we should a) require units to be specified, and b) error if the value specified is below '1 unit', but still non-zero, as it would then be rounded to zero. That would not be a back-portable fix. There are many 3rd-party tools out there (AWS RDS, for one) which do not use the units. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] proposal: rounding up time value less than its unit.
* Josh Berkus (j...@agliodbs.com) wrote: On 09/26/2014 10:27 AM, Stephen Frost wrote: I don't think anyone is argueing that we should do away with the rounding rules entirely, only that we should a) require units to be specified, and b) error if the value specified is below '1 unit', but still non-zero, as it would then be rounded to zero. That would not be a back-portable fix. No, certainly not. Sorry, thought that was clear.. There are many 3rd-party tools out there (AWS RDS, for one) which do not use the units. Of course. Those would need to be updated for whichever major release introduced this requirement. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Fri, Sep 26, 2014 at 1:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: If we want the narrowest possible fix for this, I think it's complain if a non-zero value would round to zero. That fixes the original complaint and changes absolutely nothing else. But I think that's kind of wussy. Yeah, rounding 29 seconds down to a special magic value of 0 is more surprising than rounding 30 seconds up to a minute, but the latter is still surprising. We're generally not averse to tighter validation, so why here? So in other words, if I set shared_buffers = 100KB, you are proposing that that be rejected because it's not an exact multiple of 8KB? This seems like it's throwing away one of the fundamental reasons why we invented GUC units in the first place. Both Robert and myself at one point made suggestions to this effect but I believe at this point we are both good simply solving the 1 rounding and calling it a day. I apparently have got to make this point one more time: if the user cares about the difference between 30sec and 1min, then we erred in designing the GUC in question; it should have had a smaller unit. I am completely not impressed by arguments based on such cases. The right fix for such a case is to choose a different unit for the GUC. You are right - both those values are probably quite stupid to set log_rotation_age to...but we cannot error if they choose 1min Stephen suggested changing the unit but that particular cure seems to be considerably worse than simply changing floor() to ceil() I'm out of arguments for why the minimally invasive solution is, for me, the best of the currently proposed solutions. That option gets my +1. I have to ask someone else to actually write such a patch but it seems straight forward enough as no one has complained that the solution would be hard to implement - only that they dislike the idea. David J
Re: [HACKERS] proposal: rounding up time value less than its unit.
David Johnston david.g.johns...@gmail.com writes: On Fri, Sep 26, 2014 at 1:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: I apparently have got to make this point one more time: if the user cares about the difference between 30sec and 1min, then we erred in designing the GUC in question; it should have had a smaller unit. I am completely not impressed by arguments based on such cases. The right fix for such a case is to choose a different unit for the GUC. You are right - both those values are probably quite stupid to set log_rotation_age to...but we cannot error if they choose 1min I've been thinking more about this, and I think I'm about ready to change my position on it: why shouldn't we error out if the value is too small? If we believe that a GUC's unit is reasonably chosen, then it's not sensible to try to set the value to less than 1 unit. If the user tries then there's fairly good reason to assume that either it's a typo or he fundamentally doesn't understand the GUC. (This does not imply that he cares about the difference between and .4 units.) Or another way to say it is that if we had set the min_val to something positive, he'd have gotten a value out of range type of error. The only reason we don't do that, typically, is that we're allowing zero as a special case. Well, ok, let's allow zero as a special case, but it has to be written as 0 not something else. If you try to set a positive value then we should act as though the min_val is 1 unit. So I'm coming around to the idea that throw an error if a nonzero input would round (or truncate) to zero is a reasonable solution. I think it'd be even more reasonable if we also fixed the rounding rule to be round to nearest, but the two changes can be considered independently. 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] proposal: rounding up time value less than its unit.
On Fri, Sep 26, 2014 at 2:02 PM, Robert Haas robertmh...@gmail.com wrote: On Fri, Sep 26, 2014 at 1:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: If we want the narrowest possible fix for this, I think it's complain if a non-zero value would round to zero. That fixes the original complaint and changes absolutely nothing else. But I think that's kind of wussy. Yeah, rounding 29 seconds down to a special magic value of 0 is more surprising than rounding 30 seconds up to a minute, but the latter is still surprising. We're generally not averse to tighter validation, so why here? So in other words, if I set shared_buffers = 100KB, you are proposing that that be rejected because it's not an exact multiple of 8KB? Absolutely. And if anyone is inconvenienced by that, then they should upgrade to a 386. Seriously, who is going to set a value of shared_buffers that is not measured in MB? And if they do, why shouldn't we complain if we can't honor the value exactly? If they really put in a value that small, it's not stupid to think that the difference between 96kB and 104kB is significant. Honestly, the most likely explanation for that value is that it's a developer doing testing. Related thought - why don't we allow the user to specify 1.5MB as a valid value? Since we don't the rounding on the 8kb stuff makes sense because not everyone wants to choose between 1MB and 2MB. A difference of 1 minute is not as noticeable. In the thread Tom linked to earlier the whole idea of a unit being 8kb (instead 1 block) is problematic and this is just another symptom of that. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
* Tom Lane (t...@sss.pgh.pa.us) wrote: I've been thinking more about this, and I think I'm about ready to change my position on it: why shouldn't we error out if the value is too small? If we believe that a GUC's unit is reasonably chosen, then it's not sensible to try to set the value to less than 1 unit. Right, agreed. If the user tries then there's fairly good reason to assume that either it's a typo or he fundamentally doesn't understand the GUC. (This does not imply that he cares about the difference between and .4 units.) +1 Or another way to say it is that if we had set the min_val to something positive, he'd have gotten a value out of range type of error. The only reason we don't do that, typically, is that we're allowing zero as a special case. Well, ok, let's allow zero as a special case, but it has to be written as 0 not something else. If you try to set a positive value then we should act as though the min_val is 1 unit. Yes. So I'm coming around to the idea that throw an error if a nonzero input would round (or truncate) to zero is a reasonable solution. I think it'd be even more reasonable if we also fixed the rounding rule to be round to nearest, but the two changes can be considered independently. Agreed- they're independent considerations and the original concern was about the nonzero-to-zero issue, so I'd suggest we address that first, though in doing so we will need to consider what *actual* min values we should have for some cases which currently allow going to zero for the special case and that, I believe, makes this all 9.5 material and allows us a bit more freedom in deciding how to hanlde things more generally. As such, I'd also +1 the round-to-nearest suggestion as being quite sensible too. THanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
Stephen Frost sfr...@snowman.net writes: Agreed- they're independent considerations and the original concern was about the nonzero-to-zero issue, so I'd suggest we address that first, though in doing so we will need to consider what *actual* min values we should have for some cases which currently allow going to zero for the special case and that, I believe, makes this all 9.5 material and allows us a bit more freedom in deciding how to hanlde things more generally. Yeah, I was thinking the same: we should go through the GUCs having zero as min_val and see if any of them could be tightened up. And I agree that *all* of this is 9.5 material --- it's not a big enough deal to risk changing behaviors in a minor release. 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] proposal: rounding up time value less than its unit.
On Fri, Sep 26, 2014 at 2:27 PM, Stephen Frost sfr...@snowman.net wrote: Agreed- they're independent considerations and the original concern was about the nonzero-to-zero issue, so I'd suggest we address that first, though in doing so we will need to consider what *actual* min values we should have for some cases which currently allow going to zero for the special case and that, I believe, makes this all 9.5 material and allows us a bit more freedom in deciding how to hanlde things more generally. This is 9.5 material because 1) it isn't all that critical and, 2) we DO NOT want a system to not come up because of a GUC paring error after a minor-release update. I don't get where we need to do anything else besides that...the whole actual min values comment is unclear to me. My thought on rounding is simply no-complaint, no-change; round-to-nearest would be my first choice if implementing anew. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
David, * David Johnston (david.g.johns...@gmail.com) wrote: This is 9.5 material because 1) it isn't all that critical and, 2) we DO NOT want a system to not come up because of a GUC paring error after a minor-release update. Agreed. I don't get where we need to do anything else besides that...the whole actual min values comment is unclear to me. Well, for cases that allow going to zero as an off option, we've already decided, I believe, that sub-1-unit options are off the table and so the min value is at *least* 1, but there could be cases where '1' doesn't actually make any sense and it should be higher than that. Consider the log file rotation bit. If it was in seconds, would it actually make sense to support actually doing a rotation *every second*? No. In that case, perhaps we'd set the minimum to '60s', even though technically we could represent less than that, it's not sensible to do so. The point of having minimum (and maximum..) values is that typos and other mistakes happen and we want the user to realize they've made a mistake. What needs to happen next is a review of all the GUCs which allow going to zero and which treat zero as a special value, and consider what the *actual* minimum value for those should be (excluding zero). I was hoping you might be interested in doing that... :D Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Fri, Sep 26, 2014 at 2:39 PM, Stephen Frost [via PostgreSQL] ml-node+s1045698n5820714...@n5.nabble.com wrote: David, * David Johnston ([hidden email] http://user/SendEmail.jtp?type=nodenode=5820714i=0) wrote: This is 9.5 material because 1) it isn't all that critical and, 2) we DO NOT want a system to not come up because of a GUC paring error after a minor-release update. Agreed. I don't get where we need to do anything else besides that...the whole actual min values comment is unclear to me. Well, for cases that allow going to zero as an off option, we've already decided, I believe, that sub-1-unit options are off the table and so the min value is at *least* 1, but there could be cases where '1' doesn't actually make any sense and it should be higher than that. Consider the log file rotation bit. If it was in seconds, would it actually make sense to support actually doing a rotation *every second*? No. In that case, perhaps we'd set the minimum to '60s', even though technically we could represent less than that, it's not sensible to do so. The point of having minimum (and maximum..) values is that typos and other mistakes happen and we want the user to realize they've made a mistake. What needs to happen next is a review of all the GUCs which allow going to zero and which treat zero as a special value, and consider what the *actual* minimum value for those should be (excluding zero). I was hoping you might be interested in doing that... :D Like I said I just want to fix the bug and call it a day :) For me just enforcing 1 as the minimum for everything would be fine. I'd rather mandate non-integer data entry than impose an actual minimum that is greater than 1. Specifically a too-short/too-small value might be used during exploration and testing by a new user even if the same value would never be useful in production. That, in fact, is the one reason that allowing 5s for log rotation age would make sense - to allow people to more easily checking their log rotation policies. But making it work without disrupting people using =60' (1hr) is impossible without simply outlawing unitless values. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5820717.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 09/26/2014 11:17 AM, Tom Lane wrote: So I'm coming around to the idea that throw an error if a nonzero input would round (or truncate) to zero is a reasonable solution. I think it'd be even more reasonable if we also fixed the rounding rule to be round to nearest, but the two changes can be considered independently. I'm good with the error. We'll want to add stuff to both the docs and pg_settings to *show* the minimum value, and an informative error message would help, i.e.: Invalid value for log_rotation_interval. Minimum value is 1min -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] proposal: rounding up time value less than its unit.
On 9/26/14, 2:17 PM, Tom Lane wrote: Well, ok, let's allow zero as a special case, but it has to be written as 0 not something else. If you try to set a positive value then we should act as though the min_val is 1 unit. So I'm coming around to the idea that throw an error if a nonzero input would round (or truncate) to zero is a reasonable solution. I expressed some distaste for throwing errors before, but I find this specific rule reasonable. I'll even write the patch. I owe the GUC system a re-match after my wal_buffers auto-scaling thing for 9.1 rippled to cause extra work for you (maybe Robert too). I just changed this submission from Ready for Committer to Returned with Feedback, with the feedback being that we'd like to see special values rounded to 0 treated differently altogether first, before touching any rounding. And that's a whole new submission for the next CF. I think it'd be even more reasonable if we also fixed the rounding rule to be round to nearest, but the two changes can be considered independently. regards, tom lane Personally I'm still -1 on any rounding change, instead preferring to work toward eliminating these 0 -1 special values altogether. Outside of these special cases, I feel you are fundamentally right that if the rounding really matters, the unit is simply too large. And I believe that is the case for the log rotation one right now. I can see my idea for rescaling the rotation age parameter is an unpopular one, but I still like it. That cleanly splits out into a third thing though, where it can live or die without being tied to the rest of these issues. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] proposal: rounding up time value less than its unit.
On 9/26/14, 2:34 PM, David Johnston wrote: I don't get where we need to do anything else besides that...the whole actual min values comment is unclear to me. If you look at pg_settings, there is a minimum value exposed there as min_val. For some of these parameters, that number would normally be 1. But since we have decided that 0 is a special flag value, min_val is 0 instead. There are others where min_val is -1 for the same reason, where functionally the minimum is really 0. Some of us would like to see min_val reflect the useful minimum, period, and move all these special case ones out of there. That is a multi-year battle to engage in though, and there's little real value to the user community coming out of it relative to that work scope. -- 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] proposal: rounding up time value less than its unit.
Gregory Smith gregsmithpg...@gmail.com writes: On 9/26/14, 2:34 PM, David Johnston wrote: â I don't get where we need to do anything else besides that...the whole actual min values comment is unclear to me. If you look at pg_settings, there is a minimum value exposed there as min_val. For some of these parameters, that number would normally be 1. But since we have decided that 0 is a special flag value, min_val is 0 instead. Right. There are others where min_val is -1 for the same reason, where functionally the minimum is really 0. Some of us would like to see min_val reflect the useful minimum, period, and move all these special case ones out of there. That is a multi-year battle to engage in though, and there's little real value to the user community coming out of it relative to that work scope. The impression I had was that Stephen was thinking of actually setting min_val to 1 (or whatever) and handling zero or -1 in some out-of-band fashion, perhaps by adding GUC flag bits showing those as allowable special cases. I'm not sure how we would display such a state of affairs in pg_settings, but other than that it doesn't sound implausible. We could alternatively try to split up these cases into multiple GUCs, which I guess is what you're imagining as a multi-year battle. But personally I think any such proposal will fail on the grounds that it's too much compatibility loss for the value gained. 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] proposal: rounding up time value less than its unit.
* Tom Lane (t...@sss.pgh.pa.us) wrote: There are others where min_val is -1 for the same reason, where functionally the minimum is really 0. Some of us would like to see min_val reflect the useful minimum, period, and move all these special case ones out of there. That is a multi-year battle to engage in though, and there's little real value to the user community coming out of it relative to that work scope. The impression I had was that Stephen was thinking of actually setting min_val to 1 (or whatever) and handling zero or -1 in some out-of-band fashion, perhaps by adding GUC flag bits showing those as allowable special cases. I'm not sure how we would display such a state of affairs in pg_settings, but other than that it doesn't sound implausible. Yes. I'm not 100% sure about how to deal with it in pg_settings, but that is the general idea. We could alternatively try to split up these cases into multiple GUCs, which I guess is what you're imagining as a multi-year battle. But personally I think any such proposal will fail on the grounds that it's too much compatibility loss for the value gained. Agreed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
Tom Lane wrote: The impression I had was that Stephen was thinking of actually setting min_val to 1 (or whatever) and handling zero or -1 in some out-of-band fashion, perhaps by adding GUC flag bits showing those as allowable special cases. I'm not sure how we would display such a state of affairs in pg_settings, but other than that it doesn't sound implausible. I would think that if we're going to have out of band values, we should just use off, not 0 or -1. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Friday, September 26, 2014, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Tom Lane wrote: The impression I had was that Stephen was thinking of actually setting min_val to 1 (or whatever) and handling zero or -1 in some out-of-band fashion, perhaps by adding GUC flag bits showing those as allowable special cases. I'm not sure how we would display such a state of affairs in pg_settings, but other than that it doesn't sound implausible. I would think that if we're going to have out of band values, we should just use off, not 0 or -1. Except off is not always semantically correct - some GUCs (not sure which ones ATM) use zero to mean 'default'. I think -1 always means off. Instead of 0 and -1 we'd need 'default' and 'off' with the ability to error if there is no meaningful default defined or if a feature cannot be turned off. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
David Johnston wrote: On Friday, September 26, 2014, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Tom Lane wrote: The impression I had was that Stephen was thinking of actually setting min_val to 1 (or whatever) and handling zero or -1 in some out-of-band fashion, perhaps by adding GUC flag bits showing those as allowable special cases. I'm not sure how we would display such a state of affairs in pg_settings, but other than that it doesn't sound implausible. I would think that if we're going to have out of band values, we should just use off, not 0 or -1. Except off is not always semantically correct - some GUCs (not sure which ones ATM) use zero to mean 'default'. I think -1 always means off. Instead of 0 and -1 we'd need 'default' and 'off' with the ability to error if there is no meaningful default defined or if a feature cannot be turned off. Sure, off (and other spellings of boolean false value) and default where they make sense, and whatever other values that make sense. My point is to avoid collapsing such logical values to integer/floating point values just because the option uses a numeric scale. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 9/26/14, 2:50 PM, David G Johnston wrote: Like I said I just want to fix the bug and call it a day :) I'm afraid you've come to the wrong project and mailing list for *that*. -- 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] proposal: rounding up time value less than its unit.
On Friday, September 26, 2014, Alvaro Herrera alvhe...@2ndquadrant.com wrote: David Johnston wrote: On Friday, September 26, 2014, Alvaro Herrera alvhe...@2ndquadrant.com javascript:; wrote: Tom Lane wrote: The impression I had was that Stephen was thinking of actually setting min_val to 1 (or whatever) and handling zero or -1 in some out-of-band fashion, perhaps by adding GUC flag bits showing those as allowable special cases. I'm not sure how we would display such a state of affairs in pg_settings, but other than that it doesn't sound implausible. I would think that if we're going to have out of band values, we should just use off, not 0 or -1. Except off is not always semantically correct - some GUCs (not sure which ones ATM) use zero to mean 'default'. I think -1 always means off. Instead of 0 and -1 we'd need 'default' and 'off' with the ability to error if there is no meaningful default defined or if a feature cannot be turned off. Sure, off (and other spellings of boolean false value) and default where they make sense, and whatever other values that make sense. My point is to avoid collapsing such logical values to integer/floating point values just because the option uses a numeric scale. My comment was more about storage than data entry. I'm not sure, though, that we'd want to allow 0 as valid input even if it is acceptable for Boolean. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 9/25/14, 1:41 AM, David Johnston wrote: If the error message is written correctly most people upon seeing the error will simply fix their configuration and move on - regardless of whether they were proactive in doing so having read the release notes. The important part to realize here is that most people will never see such an error message. There is a person/process who breaks the postgresql.conf, a process that asks for a configuration restart/reload (probably via pg_ctl, and then the postmaster program process creating a server log entry that shows the error (maybe in pgstartup.log, maybe in pg_log, maybe in syslog, maybe in the Windows Event Log) In practice, the top of that food chain never knows what's happening at the bottom unless something goes so seriously wrong the system is down, and they are forced to drill down into all of these log destinations. That's why a subset of us consider any error message based approaches to GUC guidance a complete waste of time. I won't even address the rest of your comments; you're focusing on trivia around something that just fundamentally isn't useful at all. My challenge to anyone who things error checking has value for this issue is to demonstrate how that would play out usefully on a mainstream Postgres system like RHEL/CentOS, Debian, or even Windows Put your bogus setting in the config file, activate that config file in a Postgres that looks for the round errors people dislike, and show me how that mistake is made apparent to the user who made it. I've done similar exercises myself, and my guess is that if the system is up at all, those error messages went by completely unnoticed. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] proposal: rounding up time value less than its unit.
On Thursday, September 25, 2014, Gregory Smith gregsmithpg...@gmail.com wrote: On 9/25/14, 1:41 AM, David Johnston wrote: If the error message is written correctly most people upon seeing the error will simply fix their configuration and move on - regardless of whether they were proactive in doing so having read the release notes. The important part to realize here is that most people will never see such an error message. There is a person/process who breaks the postgresql.conf, a process that asks for a configuration restart/reload (probably via pg_ctl, and then the postmaster program process creating a server log entry that shows the error (maybe in pgstartup.log, maybe in pg_log, maybe in syslog, maybe in the Windows Event Log) In practice, the top of that food chain never knows what's happening at the bottom unless something goes so seriously wrong the system is down, [...] And if the GUC setting here is wrong the system will be down, right? Otherwise the attempt at changing the setting will fail and so even if the message itself is not seen the desired behavior of the system will remain as it was - which is just as valid a decision rounding to zero or 1. Just like we don't take responsibility for people not reading release notes or checking their configuration if the DBA is not aware of where the GUCs are being set and the logging destinations that not our problem. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Thu, Sep 25, 2014 at 12:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: TBH I've also been wondering whether any of these proposed cures are better than the disease. I couldn't agree more. There's something to be said for just leaving this alone. The changes that can be argued to make the behavior more sane are also ones that introduce backwards compatibility issues of one magnitude or another. But on this point I think David Johnston said it best: # Any change has the potential to draw complaints. For you it seems that hey, # I upgraded to 9.5 and my logs are being rotated out every minute now. I # thought I had that turned off is the desired complaint. Greg wants: hey, my # 1 hour log rotation is now happening every minute. If the error message is # written correctly most people upon seeing the error will simply fix their # configuration and move on - regardless of whether they were proactive in # doing so having read the release notes. I particularly agree with his first sentence - any change can potentitally draw complaints. But I also agree with his last one - of those three possible complaints, I certainly prefer I had to fix my configuration file for the new, stricter validation over any variant of my configuration file still worked but it did something surprisingly different from what it used to do.. YMMV. -- 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] proposal: rounding up time value less than its unit.
* Robert Haas (robertmh...@gmail.com) wrote: On Thu, Sep 25, 2014 at 12:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: TBH I've also been wondering whether any of these proposed cures are better than the disease. I couldn't agree more. There's something to be said for just leaving this alone. I've been coming around to this also. I had thought earlier that there was consensus happening, but clearly that's not the case. The changes that can be argued to make the behavior more sane are also ones that introduce backwards compatibility issues of one magnitude or another. But on this point I think David Johnston said it best: # Any change has the potential to draw complaints. For you it seems that hey, # I upgraded to 9.5 and my logs are being rotated out every minute now. I # thought I had that turned off is the desired complaint. Greg wants: hey, my # 1 hour log rotation is now happening every minute. If the error message is # written correctly most people upon seeing the error will simply fix their # configuration and move on - regardless of whether they were proactive in # doing so having read the release notes. I particularly agree with his first sentence - any change can potentitally draw complaints. But I also agree with his last one - of those three possible complaints, I certainly prefer I had to fix my configuration file for the new, stricter validation over any variant of my configuration file still worked but it did something surprisingly different from what it used to do.. I'll agree with this also (which is why I had suggested moving forward with the idea that I thought had consensus- keep things the way they are, but toss an error if we round down a non-zero value to zero). As with Tom, I'm not against being argued to a different position, such as rounding up instead of down, but I still don't like the near-zero goes to zero situation we currently have. I'd be much happier if we'd pick one or the other and move forward with it, or agree that we can't reach a consensus and leave well enough alone. Not entirely sure what the best way to get to one of the above is, but I don't feel like we're really making much more progress at this point. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 9/25/14 11:03 AM, Robert Haas wrote: I couldn't agree more. There's something to be said for just leaving this alone. I agree. potentitally draw complaints. But I also agree with his last one - of those three possible complaints, I certainly prefer I had to fix my configuration file for the new, stricter validation over any variant of my configuration file still worked but it did something surprisingly different from what it used to do.. Yes. I don't mind that we rename parameters from time to time when semantics changed or are refined. But having the same parameter setting mean different things in different versions is the path to complete madness. -- 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] proposal: rounding up time value less than its unit.
On 9/25/14, 2:02 PM, Peter Eisentraut wrote: But having the same parameter setting mean different things in different versions is the path to complete madness. Could we go so far as to remove support for unitless time settings eventually? The fact that people are setting raw numbers in the configuration file and have to know the unit to understand what they just did has never been something I like. -- 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] proposal: rounding up time value less than its unit.
* Gregory Smith (gregsmithpg...@gmail.com) wrote: On 9/25/14, 2:02 PM, Peter Eisentraut wrote: But having the same parameter setting mean different things in different versions is the path to complete madness. Could we go so far as to remove support for unitless time settings eventually? The fact that people are setting raw numbers in the configuration file and have to know the unit to understand what they just did has never been something I like. I could certainly get behind that idea... Tho I do understand that people will complain about backwards compatibility, etc, etc. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 9/23/14 11:55 PM, Gregory Smith wrote: Right now there are people out there who have configurations that look like this: log_rotation_age=60 In order to get hourly rotation. These users will suffer some trauma should they upgrade to a version where this parameter now means a new log file pops every 60 seconds instead. But then this proposal is just one of several others that break backward compatibility, and does so in a more or less silent way. Then we might as well pick another approach that gets closer to the root of the problem. -- 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] proposal: rounding up time value less than its unit.
On 9/24/14, 6:45 PM, Peter Eisentraut wrote: But then this proposal is just one of several others that break backward compatibility, and does so in a more or less silent way. Then we might as well pick another approach that gets closer to the root of the problem. I was responding to some desire to get a quick fix that cut off the worst of the problem, and the trade-offs of the other ideas bothered me even more. Obvious breakage is annoying, but small and really subtle version to version incompatibilities drive me really crazy. A 60X scale change to log_rotation_age will be big enough that impacted users should definitely notice, while not being so big it's scary. Rounding differences are small enough to miss. And I do see whacking the sole GUC that's set in minutes, which I was surprised to find is a thing that existed at all, as a useful step forward. I can't agree with Stephen's optimism that some wording cleanup is all that's needed here. I predict it will be an annoying, multiple commit job just to get the docs right, describing both the subtle rounding change and how it will impact users. (Cry an extra tear for the translators) Meanwhile, I'd wager the entire work of my log_rotation_scale unit change idea, from code to docs to release notes, will take less time than just getting the docs done on any rounding change. Probably cause less field breakage and confusion in the end too. The bad case I threw out is a theorized one that shows why we can't go completely crazy with jerking units around, why 1000:1 adjustments are hard. I'm not actually afraid of that example being in the wild in any significant quantity. The resulting regression from a 60X scale change should not be so bad that people will suffer greatly from it either. Probably be like the big 10:1 change in default_statistics_target. Seemed scary that some people might be nailed by it; didn't turn out to be such a big deal. I don't see any agreement on the real root of a problem here yet. That makes gauging whether any smaller change leads that way or not fuzzy. I personally would be fine doing nothing right now, instead waiting until that's charted out--especially if the alternative is applying any of the rounding or error throwing ideas suggested so far. I'd say that I hate to be that guy who tells everyone else they're wrong, but we all know I enjoy it. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] proposal: rounding up time value less than its unit.
Gregory Smith gregsmithpg...@gmail.com writes: I don't see any agreement on the real root of a problem here yet. That makes gauging whether any smaller change leads that way or not fuzzy. I personally would be fine doing nothing right now, instead waiting until that's charted out--especially if the alternative is applying any of the rounding or error throwing ideas suggested so far. I'd say that I hate to be that guy who tells everyone else they're wrong, but we all know I enjoy it. TBH I've also been wondering whether any of these proposed cures are better than the disease. The changes that can be argued to make the behavior more sane are also ones that introduce backwards compatibility issues of one magnitude or another. And I do not have a lot of sympathy for let's not change anything except to throw an error in a case that seems ambiguous. That's mostly being pedantic, not helpful, especially seeing that the number of field complaints about it is indistinguishable from zero. I am personally not as scared of backwards-compatibility problems as some other commenters: I do not think that there's ever been a commitment that postgresql.conf contents will carry forward blindly across major releases. So I'd be willing to break strict compatibility in the name of making the behavior less surprising. But the solutions that have been proposed that hold to strict backwards compatibility requirements are not improvements IMHO. 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] proposal: rounding up time value less than its unit.
On Thu, Sep 25, 2014 at 12:11 AM, Gregory Smith gregsmithpg...@gmail.com wrote: On 9/24/14, 6:45 PM, Peter Eisentraut wrote: But then this proposal is just one of several others that break backward compatibility, and does so in a more or less silent way. Then we might as well pick another approach that gets closer to the root of the problem. I was responding to some desire to get a quick fix that cut off the worst of the problem, and the trade-offs of the other ideas bothered me even more. Obvious breakage is annoying, but small and really subtle version to version incompatibilities drive me really crazy. A 60X scale change to log_rotation_age will be big enough that impacted users should definitely notice, while not being so big it's scary. Rounding differences are small enough to miss. And I do see whacking the sole GUC that's set in minutes, which I was surprised to find is a thing that existed at all, as a useful step forward. Why? Do you agree that a log_rotation_age value defined in seconds is sane? If your reasoning is that everything else is defined in s and ms then that is a developer, not a user, perspective. I'll buy into the everything is defined in the smallest possible unit approach - in which case the whole rounding things becomes a non-issue - but if you leave some of these in seconds then we should still add an error if someone defines an insane millisecond value for those parameters. I can't agree with Stephen's optimism that some wording cleanup is all that's needed here. I predict it will be an annoying, multiple commit job just to get the docs right, describing both the subtle rounding change and how it will impact users. (Cry an extra tear for the translators) Maybe I'm nieve but I'm seriously doubting this. From what I can tell the rounding isn't currently documented and really doesn't need much if any to be added. An error instead of rounding down to zero would be sufficient and self-contained. The value specified is less than 1 in the parameter's base unit Meanwhile, I'd wager the entire work of my log_rotation_scale unit change idea, from code to docs to release notes, will take less time than just getting the docs done on any rounding change. Probably cause less field breakage and confusion in the end too. You've already admitted there will be breakage. Your argument is that it will be obvious enough to notice. Why not just make it impossible? The bad case I threw out is a theorized one that shows why we can't go completely crazy with jerking units around, why 1000:1 adjustments are hard. I'm not actually afraid of that example being in the wild in any significant quantity. The resulting regression from a 60X scale change should not be so bad that people will suffer greatly from it either. Probably be like the big 10:1 change in default_statistics_target. Seemed scary that some people might be nailed by it; didn't turn out to be such a big deal. I don't see any agreement on the real root of a problem here yet. That makes gauging whether any smaller change leads that way or not fuzzy. I personally would be fine doing nothing right now, instead waiting until that's charted out--especially if the alternative is applying any of the rounding or error throwing ideas suggested so far. I'd say that I hate to be that guy who tells everyone else they're wrong, but we all know I enjoy it. Maybe not: I contend the root problem is that while we provide sane unit specifications we've assumed that people will always be providing values greater than 1 - but if they don't we silently use zero (a special value) instead of telling them they messed up (made an error). If the presence of config.d and such make this untenable then I'd say those features have a problem.- not our current choice to define what sane is. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Thu, Sep 25, 2014 at 12:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: Gregory Smith gregsmithpg...@gmail.com writes: I don't see any agreement on the real root of a problem here yet. That makes gauging whether any smaller change leads that way or not fuzzy. I personally would be fine doing nothing right now, instead waiting until that's charted out--especially if the alternative is applying any of the rounding or error throwing ideas suggested so far. I'd say that I hate to be that guy who tells everyone else they're wrong, but we all know I enjoy it. TBH I've also been wondering whether any of these proposed cures are better than the disease. The changes that can be argued to make the behavior more sane are also ones that introduce backwards compatibility issues of one magnitude or another. And I do not have a lot of sympathy for let's not change anything except to throw an error in a case that seems ambiguous. That's mostly being pedantic, not helpful, especially seeing that the number of field complaints about it is indistinguishable from zero. Then what does it matter that we'd choose to error-out? I am personally not as scared of backwards-compatibility problems as some other commenters: I do not think that there's ever been a commitment that postgresql.conf contents will carry forward blindly across major releases. So I'd be willing to break strict compatibility in the name of making the behavior less surprising. But the solutions that have been proposed that hold to strict backwards compatibility requirements are not improvements IMHO. Or, put differently, the pre-existing behavior is fine so don't fix what isn't broken. This patch simply fixes an oversight in the original implementation - that someone might try to specify an invalid value (i.e., between 0 and 1). if 0 and -1 are flags, then the minimum allowable value is 1. The logic should have been: range [1, something]; 0 (optionally); -1 (optionally). Values abs(x) between 0 and 1 (exclusive) should be disallowed and, like an attempt to specify 0.5 (without units), should throw an error. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
David Johnston david.g.johns...@gmail.com writes: On Thu, Sep 25, 2014 at 12:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: TBH I've also been wondering whether any of these proposed cures are better than the disease. The changes that can be argued to make the behavior more sane are also ones that introduce backwards compatibility issues of one magnitude or another. And I do not have a lot of sympathy for let's not change anything except to throw an error in a case that seems ambiguous. That's mostly being pedantic, not helpful, especially seeing that the number of field complaints about it is indistinguishable from zero. âThen what does it matter that we'd choose to error-out?â The number of complaints about the *existing* behavior is indistinguishable from zero (AFAIR anyway). It does not follow that deciding to throw an error where we did not before will draw no complaints. 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] proposal: rounding up time value less than its unit.
On Thu, Sep 25, 2014 at 1:04 AM, Tom Lane t...@sss.pgh.pa.us wrote: David Johnston david.g.johns...@gmail.com writes: On Thu, Sep 25, 2014 at 12:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: TBH I've also been wondering whether any of these proposed cures are better than the disease. The changes that can be argued to make the behavior more sane are also ones that introduce backwards compatibility issues of one magnitude or another. And I do not have a lot of sympathy for let's not change anything except to throw an error in a case that seems ambiguous. That's mostly being pedantic, not helpful, especially seeing that the number of field complaints about it is indistinguishable from zero. Then what does it matter that we'd choose to error-out? The number of complaints about the *existing* behavior is indistinguishable from zero (AFAIR anyway). It does not follow that deciding to throw an error where we did not before will draw no complaints. Any change has the potential to draw complaints. For you it seems that hey, I upgraded to 9.5 and my logs are being rotated out every minute now. I thought I had that turned off is the desired complaint. Greg wants: hey, my 1 hour log rotation is now happening every minute. If the error message is written correctly most people upon seeing the error will simply fix their configuration and move on - regardless of whether they were proactive in doing so having read the release notes. And, so what if we get some complaints? We already disallow the specified values (instead, turning them into zeros) so it isn't like we are further restricting user capabilities. If I was to commit a patch that didn't throw an error I'd ask the author to provide an outline for each of the affected parameters and what it would mean (if possible) for a setting currently rounded to zero to instead be rounded to 1. Its the same language in the release notes to get someone to avoid the error as it is to get them to not be impacted by the rounding change so the difference is those people who would not read the release notes. The error outcome is simple, direct, and fool-proof - and conveys the fact that what they are asking for is invalid. It may be a little scary but at least we can be sure nothing bad is happening in the system. If the argument is that there are two few possible instances out there to expend the effort to catalog every parameter then there isn't enough surface area to care about throwing an error. And I still maintain that anyone setting values for a clean installation would rather be told their input is not valid instead of silently making it so. Changing the unit for log_rotate_age when their is basically no one asking for that seems like the worse solution at face value; my +1 not withstanding. I gave that mostly because if you are going to overhaul the system then making everything ms seems like the right thing to do. I think that such an effort would be a waste of resources. I don't have clients so maybe my acceptance of errors is overly optimistic - but in this case I truly don't see enough people even realizing that there was a change and those few that do should be able to read the error and make the same change that they would need to make anyway if the rounding option is chosen. My only concern here, and it probably applies to any solution, is that the change is implemented properly so that all possible user input areas throw the error correctly and as appropriate to the provided interface. That is, interactive use immediately errors out without changing the default values while explicit/manual file changes cause the system to error at startup. I haven't looked into the code parts of this but I have to imagine this is already covered since even with units and such invalid input is still possible and needs to be addressed; this only add one more check to that existing routine. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Tue, Sep 23, 2014 at 1:23 AM, Stephen Frost sfr...@snowman.net wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: To clarify- we'll simply swap from (essentially) floor() to ceil() for handling all GUC_with_unit to internal_unit conversions, document that, and note it in the release notes as a possible behavior change and move on. Worksforme. Great, thanks. I'll wait a couple days to see if anyone else wants to voice a concern. Tomonari, don't worry about updating the patch (unless you really want to) as I suspect I'll be changing around the wording anyway. No offense, please, but I suspect English isn't your first language and it'll be simpler for me if I just handle it. Thanks a lot for the bug report and discussion and I'll be sure to credit you for it in the commit. Three people have voted for making it an *error* to supply a value that needs to be rounded, instead of changing the rounding behavior. There are not three votes for any other proposal. (This may still be an improvement over the current behavior, though.) -- 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] proposal: rounding up time value less than its unit.
Robert Haas robertmh...@gmail.com writes: Three people have voted for making it an *error* to supply a value that needs to be rounded, instead of changing the rounding behavior. Votes or no votes, that's a horrible idea; it breaks the design goal that users shouldn't need to remember the precise unit size when making postgresql.conf entries. And I'm not sure what votes you're counting, anyway. People's opinions have changed as the discussion proceeded ... 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] proposal: rounding up time value less than its unit.
On Tue, Sep 23, 2014 at 10:29 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Three people have voted for making it an *error* to supply a value that needs to be rounded, instead of changing the rounding behavior. Votes or no votes, that's a horrible idea; it breaks the design goal that users shouldn't need to remember the precise unit size when making postgresql.conf entries. Not at all. You can still supply the value in another unit as long as it converts exactly. If it doesn't, shouldn't you care about that? And I'm not sure what votes you're counting, anyway. People's opinions have changed as the discussion proceeded ... David Johnston, Peter Eisentraut, myself. I don't see any indication that any of those three people have reversed their opinion at any point. -- 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] proposal: rounding up time value less than its unit.
On 09/23/2014 10:29 AM, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: Three people have voted for making it an *error* to supply a value that needs to be rounded, instead of changing the rounding behavior. Votes or no votes, that's a horrible idea; it breaks the design goal that users shouldn't need to remember the precise unit size when making postgresql.conf entries. And I'm not sure what votes you're counting, anyway. People's opinions have changed as the discussion proceeded ... I don't think I've weighed in on this, and yes, I'm very late to the party. The round away from zero suggestion seemed the simplest and most easily understood rule to me. As Tom, I think, remarked, if that seems silly because 1 second gets rounded up to 1 hour or whatever, then we've chosen the wrong units in the first place. cheers andrew -- 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] proposal: rounding up time value less than its unit.
On Tue, Sep 23, 2014 at 1:30 AM, Tom Lane t...@sss.pgh.pa.us wrote: David Johnston david.g.johns...@gmail.com writes: My original concern was things that are rounded to zero now will not be in 9.5 if the non-error solution is chosen. The risk profile is extremely small but it is not theoretically zero. This is exactly the position I was characterizing as an excessively narrow-minded attachment to backwards compatibility. We are trying to make the behavior better (as in less confusing), not guarantee that it's exactly the same. I am going to assume that the feature designers were focused on wanting to avoid: 1000 * 60 * 5 to get a 5-minute value set on a millisecond unit parameter. The designer of the variable, in choosing a unit, has specified the minimum value that they consider sane. Attempting to record an insane value should throw an error. I do not support throwing an error on all attempts to round but specifying a value less than 1 in the variable's unit should not be allowed. If such a value is proposed the user either made an error OR they misunderstand the variable they are using. In either case telling them of their error is more friendly than allowing them to discover the problem on their own. If we are only allowed to change the behavior by throwing errors in cases where we previously didn't, then we are voluntarily donning a straitjacket that will pretty much ensure Postgres doesn't improve any further. I'm not proposing project-level policy here. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 09/23/2014 06:23 PM, Andrew Dunstan wrote: On 09/23/2014 10:29 AM, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: Three people have voted for making it an *error* to supply a value that needs to be rounded, instead of changing the rounding behavior. Votes or no votes, that's a horrible idea; it breaks the design goal that users shouldn't need to remember the precise unit size when making postgresql.conf entries. And I'm not sure what votes you're counting, anyway. People's opinions have changed as the discussion proceeded ... I don't think I've weighed in on this, and yes, I'm very late to the party. The round away from zero suggestion seemed the simplest and most easily understood rule to me. +1, rounding up seems better to me as well. Although throwing an error wouldn't be that bad either. - Heikki -- 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] proposal: rounding up time value less than its unit.
Fwiw I agree with TL2. The simplest, least surprising behaviour to explain to users is to say we round to nearest and if that means we rounded to zero (or another special value) we throw an error. We could list the minimum value in pg_settings and maybe in documentation. -- greg
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Tue, Sep 23, 2014 at 3:05 PM, Greg Stark st...@mit.edu wrote: Fwiw I agree with TL2. The simplest, least surprising behaviour to explain to users is to say we round to nearest and if that means we rounded to zero (or another special value) we throw an error. We could list the minimum value in pg_settings and maybe in documentation. I'm not sure TL2 would agree that you are agreeing with him... To the other point the minimum unit-less value is 1. The unit that is applied is already listed in pg_settings and the documentation. While 0 is allowed it is more of a flag than a value. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 9/23/14 10:29 AM, Tom Lane wrote: Votes or no votes, that's a horrible idea; it breaks the design goal that users shouldn't need to remember the precise unit size when making postgresql.conf entries. I think this is not historically correct. The original motivation was that you had to remember what the units on shared_buffers = 37 were, and that it was different units than work_mem = 37 That's independent of the question whether shared_buffers = 250kB might be rejected or not. -- 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] proposal: rounding up time value less than its unit.
On 9/23/14 1:13 AM, Stephen Frost wrote: To clarify- we'll simply swap from (essentially) floor() to ceil() for handling all GUC_with_unit to internal_unit conversions, document that, and note it in the release notes as a possible behavior change and move on. That'll probably work, as long as we don't invent any other-than-zero values that are somehow treated special. One might be able to construct a case where something gets rounded to -1 when it didn't before or the other way around, but that's clearly a silly case. -- 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] proposal: rounding up time value less than its unit.
Peter Eisentraut pete...@gmx.net writes: On 9/23/14 10:29 AM, Tom Lane wrote: Votes or no votes, that's a horrible idea; it breaks the design goal that users shouldn't need to remember the precise unit size when making postgresql.conf entries. I think this is not historically correct. The original motivation was that you had to remember what the units on shared_buffers = 37 were, and that it was different units than work_mem = 37 Right, but the issue of not requiring the specified value to be an exact multiple of the underlying unit did come up in the early discussion, and we were pretty clear that we didn't want to throw an error for that: http://www.postgresql.org/message-id/flat/29818.1153976...@sss.pgh.pa.us#29818.1153976...@sss.pgh.pa.us 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] proposal: rounding up time value less than its unit.
On 9/23/14, 1:21 AM, David Johnston wrote:
This patch should fix the round-to-zero issue. If someone wants to
get rid of zero as a special case let them supply a separate patch for
that improvement.
I am going to wander into this fresh after just reading everything once
(but with more than a little practice with real-world GUC mucking) and
say that, no, it should not even do that. The original complaint here
is real and can be straightforward to fix, but I don't like any of the
proposals so far.
My suggestion: fix the one really bad one in 9.5 with an adjustment to
the units of log_rotation_age. That's a small commit that should thank
Tomonari Katsumata for discovering the issue and even suggesting a fix
(that we don't use) and a release note; sample draft below. Stop there.
Let someone with a broader objection take on the fact that zero (and -1)
values have special properties, and that sucks, as a fully reasoned
redesign. I have a larger idea for minimizing rounding issues of
timestamps in particular at the bottom of this too. I wouldn't dare
take on changes to rounding of integers without sorting out the special
flag value issue first, because the variety of those in the database is
large compared to the time ones.
== log_rotation_age ==
Back to where this started at
http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp :
log_rotation_age would be disabled if we set it less than one minute,
with this example of a surprising behavior:
log_rotation_age = 10s
postgres=# show log_rotation_age ;
log_rotation_age
--
0
That's bad and the GUC system is not being friendly; fully agreed. But
this is just one where the resolution for the parameter is poor.
log_rotation_age is the *only* GUC with a time interval that's set in
minutes:
postgres=# SELECT name, unit FROM pg_settings WHERE unit ='min';
name | unit
--+--
log_rotation_age | min
For comparison, there are 10 GUCs set in seconds and 13 in ms in HEAD.
We could just change the units for log_rotation_age to seconds, then let
the person who asks for a 10s rotation time suffer for that decision
only with many log files. The person who instead chooses 10ms may find
log rotation disabled altogether because that rounds to zero, but now we
are not talking about surprises on something that seems sane--that's a
fully unreasonable user request.
Following that style of fix all the way through to the sort of release
notes needed would give something like this:
log_rotation_age is now set in units of seconds instead of minutes.
Earlier installations of PostgreSQL that set this value directly, to a
value in minutes, should change that setting to use a time unit before
migrating to this version.
And we could be done for now.
== Flag values and error handling ==
I would like to see using 0 and -1 as special values in GUCs overhauled
one day, with full disregard for backward compatibility as a
straightjacket on doing the right thing. This entire class of behavior
is annoying. But I am skeptical anything less than such an overhaul
will really be useful. To me it's not worth adding new code,
documentation, and associated release notes to guide migration all to
try and describe new rounding trivia--which I don't see as better nor
worse than the trade-offs of what happens today.
I can't take the idea of throwing errors for something this minor
seriously at all. There are a lot more precedents for spreading
settings around in a few places now, from include_dir to
postgresql.auto.conf. There are so many ways to touch a config now and
not notice the error message now, I really don't want to see any more
situations appear where trying to change a setting will result in a
broken file added into that mix. None of this broken units due to
rounding stuff that I've found, now that I went out of my way looking
for some, even comes close to rising to that level of serious to me.
Only this log rotation one is so badly out of line that it will act
quite badly.
== Overhauling all time unit GUCs ==
Here's the complete list of potential ugly time settings to review in
the future, if my suggestion of only fixing log_rotation_age were followed:
gsmith=# SELECT name,setting, unit, min_val FROM pg_settings where unit
in ('s','ms') and (min_val::integer)=0 order by unit,min_val,name;
name | setting | unit | min_val
--+-+--+-
autovacuum_vacuum_cost_delay | 20 | ms | -1
log_autovacuum_min_duration | -1 | ms | -1
log_min_duration_statement | -1 | ms | -1
max_standby_archive_delay| 3 | ms | -1
max_standby_streaming_delay | 3 | ms | -1
lock_timeout | 0 | ms | 0
statement_timeout| 0 | ms | 0
vacuum_cost_delay| 0 | ms | 0
wal_receiver_timeout | 6
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Tue, Sep 23, 2014 at 10:11 PM, Gregory Smith gregsmithpg...@gmail.com wrote: On 9/23/14, 1:21 AM, David Johnston wrote: This patch should fix the round-to-zero issue. If someone wants to get rid of zero as a special case let them supply a separate patch for that improvement. I am going to wander into this fresh after just reading everything once (but with more than a little practice with real-world GUC mucking) and say that, no, it should not even do that. The original complaint here is real and can be straightforward to fix, but I don't like any of the proposals so far. My suggestion: fix the one really bad one in 9.5 with an adjustment to the units of log_rotation_age. That's a small commit that should thank Tomonari Katsumata for discovering the issue and even suggesting a fix (that we don't use) and a release note; sample draft below. Stop there. +1 I'm not convinced minute is wrong but it does imply a level of user-friendliness (or over-parenting) that we can do away with. We could just change the units for log_rotation_age to seconds, then let the person who asks for a 10s rotation time suffer for that decision only with many log files. The person who instead chooses 10ms may find log rotation disabled altogether because that rounds to zero, but now we are not talking about surprises on something that seems sane--that's a fully unreasonable user request. Given the following why not just pick ms for log_rotation_age now? Following that style of fix all the way through to the sort of release notes needed would give something like this: log_rotation_age is now set in units of seconds instead of minutes. Earlier installations of PostgreSQL that set this value directly, to a value in minutes, should change that setting to use a time unit before migrating to this version. ? are there any special considerations for people using pg_dump vs. those using pg_upgrade? If I were feeling ambitious about breaking configurations with a long-term eye toward improvement, I'd be perfectly happy converting everything on this list to ms. We live in 64 bit integer land now; who cares if the numbers are bigger? Then the rounding issues only impact sub-millisecond values, making all them squarely in nonsense setting land. Users will be pushed very clearly to always use time units in their postgresql.conf files instead of guessing whether something is set in ms vs. seconds. Seeing the reaction to a units change on log_rotation_age might be a useful test for how that sort of change plays out in the real world. If we are going to go that far why not simply modify the GUC input routine to require unit-values on these particular parameters? Direct manipulation of pg_settings probably would need some attention but everything else could reject integers and non-unit-specifying text. Allow the same input routine to accept the constants on|off|default and convert them internally into whatever the given GUC requires and you get the UI benefits without mucking around with the implementation details. Modify pg_settings accordingly to hide those now-irrelevant pieces. For UPDATE a trigger can be used to enforce the text-only input requirement. As long as we do not make microsecond µs a valid time-unit it is impossible currently to directly specify a fraction of a millisecond. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 9/23/14, 10:52 PM, David Johnston wrote: Given the following why not just pick ms for log_rotation_age now? Right now there are people out there who have configurations that look like this: log_rotation_age=60 In order to get hourly rotation. These users will suffer some trauma should they upgrade to a version where this parameter now means a new log file pops every 60 seconds instead. If they didn't catch the warning in the release notes and it happens, I'm pretty comfortable they'll survive though, just with a bunch of cursing until it's resolved. I'd even wager a beer that more than 10% of PostgreSQL installs that get hit won't even notice. I'd prefer not to make that experience into one where they get a log file every 60ms though. That's seriously bad. I'm not opposed to making major changes like that, I just like them to be as part of updates big enough that people are unlikely to miss that something is different. Just doing this log_rotation_age thing is small enough that I expect people to miss the change in the release notes. That limits how much I think we can change the magnitude of an easy to miss setting with a large unit adjustment. ? are there any special considerations for people using pg_dump vs. those using pg_upgrade? I don't know that there's any code in pg_upgrade aiming at this sort of job. I'd prefer not to add find parameters that have changed in meaning and flag them to pg_upgrade's job requirements. It has enough problems to worry about, and we really don't do this very often. If we are going to go that far why not simply modify the GUC input routine to require unit-values on these particular parameters? Direct manipulation of pg_settings probably would need some attention but everything else could reject integers and non-unit-specifying text. That would be fine by me as a long-term direction, but it's more of a two to three version release level of change. To keep that from being terrible for users, we'd need to provide a transition release that helped people find the problem ones without units. After that was in the wild for a while, then could we have some confidence that enough people had flushed the issue out enough to turn it into a hard requirement. The project went through this exact sort of exercise with the standard_conforming_strings GUC being the way we flagged the bad constructs for people. That was a much harder job because it touched everyone's application code too. But it took many years to play out. I'd be shocked if we could herd our flock of old time users fast enough to remove unitless GUC values from PostgreSQL in less than 3 years worth of new releases. -- Greg Smith greg.sm...@crunchydatasolutions.com Chief PostgreSQL Evangelist - http://crunchydatasolutions.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] proposal: rounding up time value less than its unit.
Tomonari, * Tomonari Katsumata (t.katsumata1...@gmail.com) wrote: I'm thinking about a method which users get quick awareness it. Now, it's okay not to change current behavior except non-zero value yields a zero. A zero rounded down from non-zero gets an error. I attached new patch. This includes a document about above behavior as Heikki suggested. Thanks for the updated patch! I was going through it and there's a few things- - Documentation change no longer applies - Why aren't we worried about a specification of '7kB' being rounded down to zero for GUCs which expect at least BLCKSZ? If the reason is everything that works that way will error on zero anyway today, then I don't buy into that argument- there's no sense leaving it inconsistent and it would be a potential land-mine to hit later. - The hint messages look like they need some rewording, at least. In general, I'm a fan of this change and will move forward with it, with changes for the above, unless folks object. Based on the thread so far, this sounds like the right solution and it'd be great to get this simple change done to help move the CF along. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
Stephen Frost sfr...@snowman.net writes: * Tomonari Katsumata (t.katsumata1...@gmail.com) wrote: I'm thinking about a method which users get quick awareness it. Now, it's okay not to change current behavior except non-zero value yields a zero. A zero rounded down from non-zero gets an error. In general, I'm a fan of this change and will move forward with it, with changes for the above, unless folks object. Based on the thread so far, this sounds like the right solution and it'd be great to get this simple change done to help move the CF along. Hm, I object to a patch that behaves as stated above. I'm okay with silently rounding *up* to the lowest possible nonzero value, eg rounding up 7kB to 8kB if the variable's unit is 8kB. But if we throw an error for 7kB and not 8kB, then we are exposing the unit size in a way that the user can't ignore. That seems to me to be antithetical to the entire design rationale for GUC units. More, it doesn't fix the original complaint here, which is that the user would be surprised if he specifies 7kB and gets some special behavior instead because it's too close to zero but not exactly zero. 7kB should act as though it's not zero. If the difference between 7kB and 8kB is actually user-meaningful, then we chose too coarse a unit for the particular GUC, which is not something that a rounding rule can paper over. But the difference between zero and not-zero is meaningful regardless of unit choices. This argument doesn't say anything much about which way to round for values that are fractional but larger than the unit size. I'd probably prefer a round away from zero behavior since that seems to be the most consistent rule if we want to avoid silently creating zero values; but I could be talked into something else. 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] proposal: rounding up time value less than its unit.
Hey Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: * Tomonari Katsumata (t.katsumata1...@gmail.com) wrote: I'm thinking about a method which users get quick awareness it. Now, it's okay not to change current behavior except non-zero value yields a zero. A zero rounded down from non-zero gets an error. In general, I'm a fan of this change and will move forward with it, with changes for the above, unless folks object. Based on the thread so far, this sounds like the right solution and it'd be great to get this simple change done to help move the CF along. Hm, I object to a patch that behaves as stated above. I'm okay with silently rounding *up* to the lowest possible nonzero value, eg rounding up 7kB to 8kB if the variable's unit is 8kB. But if we throw an error for 7kB and not 8kB, then we are exposing the unit size in a way that the user can't ignore. That seems to me to be antithetical to the entire design rationale for GUC units. More, it doesn't fix the original complaint here, which is that the user would be surprised if he specifies 7kB and gets some special behavior instead because it's too close to zero but not exactly zero. 7kB should act as though it's not zero. If the difference between 7kB and 8kB is actually user-meaningful, then we chose too coarse a unit for the particular GUC, which is not something that a rounding rule can paper over. But the difference between zero and not-zero is meaningful regardless of unit choices. I agree that the difference between 7kB and 8kB shouldn't be meaningful, but that it can be if it means we end up at zero. Having different rounding rules at near-zero than in other cases doesn't strike me as great either though, which is why I thought the error-if-rounded-to-zero made sense. As for the user, I'd aruge that they haven't understood the GUC if they're setting the value down to something which rounds to zero and an error (yes, even one in the logs that we know few enough folks read) would be better than where we are currently. This argument doesn't say anything much about which way to round for values that are fractional but larger than the unit size. I'd probably prefer a round away from zero behavior since that seems to be the most consistent rule if we want to avoid silently creating zero values; but I could be talked into something else. PeterE argued that rounding away from zero didn't make sense either (53f6501b.3090...@gmx.net). I feel like we're getting trapped by examples. Here's another proposal- how about we support a 'minimum-if-not-zero' option for GUCs more generally, and then throw an error if the user sets the value to a value below that minimum unless they explicitly use zero (to indicate whatever the special meaning of zero for that GUC is)? It'd be a larger change, certainly, but I feel like that combined with the current behavior would address this and possibly other issues (setting to a value which is low enough to be accepted, but too low to allow the system to function properly..). Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
Stephen Frost sfr...@snowman.net writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: This argument doesn't say anything much about which way to round for values that are fractional but larger than the unit size. I'd probably prefer a round away from zero behavior since that seems to be the most consistent rule if we want to avoid silently creating zero values; but I could be talked into something else. PeterE argued that rounding away from zero didn't make sense either (53f6501b.3090...@gmx.net). I feel like we're getting trapped by examples. I don't find anything compelling in Peter's argument. I do agree that if the GUC unit is hours, and the user tries to set it to 1 second or 1 microsecond, then he probably didn't understand the GUC ... but by that argument, if the unit is hours and he tries to set it to 1.001 hours, we should still throw an error. The point of the GUC units system is to not be too picky about what the variable's units are, and that that should be possible as long as the unit is small enough that the user shouldn't care about the difference between N and N+1 units. Therefore, I am entirely unimpressed by examples that hinge on the assumption that the user *does* care about that; any such example can be rejected on the grounds that it was our error to use too large a unit for that GUC. However, as long as we have any cases with special behavior for zero, we should expect that the user cares about the difference between 0 units and 1 unit. Here's another proposal- how about we support a 'minimum-if-not-zero' option for GUCs more generally, and then throw an error if the user sets the value to a value below that minimum unless they explicitly use zero (to indicate whatever the special meaning of zero for that GUC is)? I don't think that the extra complexity in that is worth the effort. You could maybe talk me into round to nearest, and then complain if that produced zero from nonzero (in effect, your proposal with the minimum always exactly half a unit). But that seems like mostly extra complexity and an extra error case for not much. Again, *the user shouldn't care* what our rounding rule is exactly; if he does, it means the particular GUC was misdesigned. We could adopt a straightforward round to nearest rule if we changed things around enough so that zero was never special, which I think is what Peter was really arguing for in the post you cite. But that strikes me as a large amount of work, and a large loss of backwards compatibility, in return for (again) not much. 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] proposal: rounding up time value less than its unit.
Tom Lane-2 wrote The case where this argument falls down is for special values, such as where zero means something quite different from the smallest nonzero value. Peter suggested upthread that we should redefine any GUC values for which that is true, but (a) I think that loses on backwards compatibility grounds, and (b) ISTM zero is probably always special to some extent. A zero time delay for example is not likely to work. Maybe we should leave the rounding behavior alone (there's not much evidence that rounding in one direction is worse than another; although I'd also be okay with changing to round-to-nearest), and confine ourselves to throwing an error for the single case that an apparently nonzero input value is truncated/rounded to zero as a result of units conversion. Tom, Can you either change your mind back to this opinion you held last month or commit something you find acceptable - its not like anyone would revert something you commit... :) Everyone agrees non-zero must not round to zero; as long as that happens I'm not seeing anyone willing to spending any more effort on the details. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5820024.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] proposal: rounding up time value less than its unit.
David G Johnston david.g.johns...@gmail.com writes: Can you either change your mind back to this opinion you held last month or commit something you find acceptable - its not like anyone would revert something you commit... :) Hey, am I not allowed to change my mind :-) ? Seriously though, the main point I was making before stands: if the details of the rounding rule matter, then we messed up on choosing the units of the particular GUC. The question is what are we going to do about zero being a special case. Everyone agrees non-zero must not round to zero; as long as that happens I'm not seeing anyone willing to spending any more effort on the details. I'm not entirely sure Peter agrees; he wanted to get rid of zero being a special case, rather than worry about making the rounding rule safe for that case. But assuming that that's a minority position: it seems to me that adding a new error condition is more work for us, and more work for users too, and not an especially sane decision when viewed from a green-field perspective. My proposal last month was in response to some folk who were arguing for a very narrow-minded definition of backwards compatibility ... but I don't think that's really where we should go. 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] proposal: rounding up time value less than its unit.
* Tom Lane (t...@sss.pgh.pa.us) wrote: Here's another proposal- how about we support a 'minimum-if-not-zero' option for GUCs more generally, and then throw an error if the user sets the value to a value below that minimum unless they explicitly use zero (to indicate whatever the special meaning of zero for that GUC is)? I don't think that the extra complexity in that is worth the effort. Alright. You could maybe talk me into round to nearest, and then complain if that produced zero from nonzero (in effect, your proposal with the minimum always exactly half a unit). But that seems like mostly extra complexity and an extra error case for not much. Again, *the user shouldn't care* what our rounding rule is exactly; if he does, it means the particular GUC was misdesigned. I agree that the user shouldn't have to care, and I agree with your earlier comment on this thread that having the rounding rules be different when near zero vs. not-near-zero could be quite confusing. We could adopt a straightforward round to nearest rule if we changed things around enough so that zero was never special, which I think is what Peter was really arguing for in the post you cite. But that strikes me as a large amount of work, and a large loss of backwards compatibility, in return for (again) not much. Agreed- I'm a bit concerned about backwards compatibility for all of the proposals which change the existing rounding rules, but, if the consensus is that N vs. N+1 shouldn't actually matter for values of N X N+1 (as a one-unit step should be small enough to be not terribly noticeable), then perhaps we can still move forward with the ceil() approach as that looks to be the only argument against it. To clarify- we'll simply swap from (essentially) floor() to ceil() for handling all GUC_with_unit to internal_unit conversions, document that, and note it in the release notes as a possible behavior change and move on. Thoughts? Objections? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
Stephen Frost sfr...@snowman.net writes: To clarify- we'll simply swap from (essentially) floor() to ceil() for handling all GUC_with_unit to internal_unit conversions, document that, and note it in the release notes as a possible behavior change and move on. Worksforme. 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] proposal: rounding up time value less than its unit.
On Tuesday, September 23, 2014, Tom Lane t...@sss.pgh.pa.us wrote: David G Johnston david.g.johns...@gmail.com javascript:; writes: Can you either change your mind back to this opinion you held last month or commit something you find acceptable - its not like anyone would revert something you commit... :) Hey, am I not allowed to change my mind :-) ? Seriously though, the main point I was making before stands: if the details of the rounding rule matter, then we messed up on choosing the units of the particular GUC. The question is what are we going to do about zero being a special case. Everyone agrees non-zero must not round to zero; as long as that happens I'm not seeing anyone willing to spending any more effort on the details. I'm not entirely sure Peter agrees; he wanted to get rid of zero being a special case, rather than worry about making the rounding rule safe for that case. But assuming that that's a minority position: it seems to me that adding a new error condition is more work for us, and more work for users too, and not an especially sane decision when viewed from a green-field perspective. My proposal last month was in response to some folk who were arguing for a very narrow-minded definition of backwards compatibility ... but I don't think that's really where we should go. regards, tom lane This patch should fix the round-to-zero issue. If someone wants to get rid of zero as a special case let them supply a separate patch for that improvement. My original concern was things that are rounded to zero now will not be in 9.5 if the non-error solution is chosen. The risk profile is extremely small but it is not theoretically zero. David J.
Re: [HACKERS] proposal: rounding up time value less than its unit.
* Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: To clarify- we'll simply swap from (essentially) floor() to ceil() for handling all GUC_with_unit to internal_unit conversions, document that, and note it in the release notes as a possible behavior change and move on. Worksforme. Great, thanks. I'll wait a couple days to see if anyone else wants to voice a concern. Tomonari, don't worry about updating the patch (unless you really want to) as I suspect I'll be changing around the wording anyway. No offense, please, but I suspect English isn't your first language and it'll be simpler for me if I just handle it. Thanks a lot for the bug report and discussion and I'll be sure to credit you for it in the commit. Thanks again! Stephen signature.asc Description: Digital signature
Re: [HACKERS] proposal: rounding up time value less than its unit.
David Johnston david.g.johns...@gmail.com writes: My original concern was things that are rounded to zero now will not be in 9.5 if the non-error solution is chosen. The risk profile is extremely small but it is not theoretically zero. This is exactly the position I was characterizing as an excessively narrow-minded attachment to backwards compatibility. We are trying to make the behavior better (as in less confusing), not guarantee that it's exactly the same. If we are only allowed to change the behavior by throwing errors in cases where we previously didn't, then we are voluntarily donning a straitjacket that will pretty much ensure Postgres doesn't improve any further. It's important here that this behavior change is being proposed for a new major release, not for back-patching. We have never supposed that postgresql.conf files had to work without any change in new major releases. 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] proposal: rounding up time value less than its unit.
Hi, I'm sorry for slow reaction. I don't care whether rounding up or down it, although this title has 'rounding up'. (I just only come up with it. I'm sorry for my imprudence) I'm thinking about a method which users get quick awareness it. Now, it's okay not to change current behavior except non-zero value yields a zero. A zero rounded down from non-zero gets an error. I attached new patch. This includes a document about above behavior as Heikki suggested. regards, -- Tomonari Katsumata 2014-08-27 6:49 GMT+09:00 David G Johnston david.g.johns...@gmail.com: Tom Lane-2 wrote Robert Haas lt; robertmhaas@ gt; writes: I liked David Johnston's even stronger suggestion upthread: make it an error to specify a value requires rounding of any kind. In other words, if the minimum granularity is 1 minute, you can specify that as 60 seconds instead, but if you write 59 seconds, we error out. Maybe that seems pedantic, but I don't think users will much appreciate the discovery that 30 seconds means 60 seconds. They'll be happier to be told that up front than having to work it out afterward. I think this is totally wrong. The entire point of the GUC units system, or at least a large part of the point, is that users should not have to be intimately aware of the units in which a given value is measured internally. And that in turn means that the units had better be such that users won't find them overly coarse. If it matters a lot whether 59 seconds gets rounded to 60, then we didn't make a good choice of units for the GUC in question; and we should fix that choice, not mess with the rounding rules. The case where this argument falls down is for special values, such as where zero means something quite different from the smallest nonzero value. Peter suggested upthread that we should redefine any GUC values for which that is true, but (a) I think that loses on backwards compatibility grounds, and (b) ISTM zero is probably always special to some extent. A zero time delay for example is not likely to work. Maybe we should leave the rounding behavior alone (there's not much evidence that rounding in one direction is worse than another; although I'd also be okay with changing to round-to-nearest), and confine ourselves to throwing an error for the single case that an apparently nonzero input value is truncated/rounded to zero as a result of units conversion. To Andres' point: SELECT unit, count(*) FROM pg_settings WHERE unit '' GROUP BY unit; (9.3 / Ubuntu) min (1 - log_rotation_age) s (10) ms (13) kb (7) 8kb (6) I don't know about the size implications but they seem to be non-existent. That any setting critically matters at +/- 1s or 1ms doesn't seem likely in practice. Even +/- 1min for a setting, if it did matter at extreme scale, would be recognizable by the user in practice as a rounding artifact and compensated for. At this point throwing an error for any precision that results in less than the default precision is my preference. I would not change the rounding rules for the simple reason that there is no obvious improvement to be had and so why introduce pointless change that - however marginal and unlikely - will be user-visible. The complaint to overcome is avoiding an interpretation of zero when the precision of the input is less than the GUC unit. Lacking any concrete complaints about our round-down policy I don't see where a change there is worthwhile. Fixing zero as a special value falls under the same category. As mathematically pure as using infinity may be the trade-off for practicality and usability seems, even in light of this complaint, like the correct one to have made. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816409.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 error_for_less-than_required_time-unit.patch Description: Binary data -- 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] proposal: rounding up time value less than its unit.
On Sat, Aug 23, 2014 at 6:39 PM, Greg Stark st...@mit.edu wrote: On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or make it an error to specify a value that rounds to 0 but isn't 0. I liked David Johnston's even stronger suggestion upthread: make it an error to specify a value requires rounding of any kind. In other words, if the minimum granularity is 1 minute, you can specify that as 60 seconds instead, but if you write 59 seconds, we error out. Maybe that seems pedantic, but I don't think users will much appreciate the discovery that 30 seconds means 60 seconds. They'll be happier to be told that up front than having to work it out afterward. -- 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] proposal: rounding up time value less than its unit.
On 2014-08-26 16:16:32 -0400, Robert Haas wrote: On Sat, Aug 23, 2014 at 6:39 PM, Greg Stark st...@mit.edu wrote: On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or make it an error to specify a value that rounds to 0 but isn't 0. I liked David Johnston's even stronger suggestion upthread: make it an error to specify a value requires rounding of any kind. In other words, if the minimum granularity is 1 minute, you can specify that as 60 seconds instead, but if you write 59 seconds, we error out. Maybe that seems pedantic, but I don't think users will much appreciate the discovery that 30 seconds means 60 seconds. They'll be happier to be told that up front than having to work it out afterward. Is the whole topic actually practically relevant? Afaics there's no guc's with a time unit bigger than GUC_UNIT_S except log_rotation_age where it surely doesn't matter and no space unit bigger than GUC_UNIT_BLOCKS/GUC_UNIT_XBLOCKS. In theory I agree with you/$subject, but I don't really see this worth much effort. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] proposal: rounding up time value less than its unit.
On 8/23/14 6:39 PM, Greg Stark wrote: Or make it an error to specify a value that rounds to 0 but isn't 0. That's what I would prefer. -- 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] proposal: rounding up time value less than its unit.
On 8/26/14 4:22 PM, Andres Freund wrote: Is the whole topic actually practically relevant? It's clearly not all that important, or otherwise we'd have heard about before now. I suppose someone could do something like wal_receiver_status_interval = 10ms and end up silently turning the whole thing off instead of making it very aggressive. The mistake here is that the mathematically appropriate turn-off value in this and similar cases is infinity, not zero. -- 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] proposal: rounding up time value less than its unit.
Robert Haas robertmh...@gmail.com writes: I liked David Johnston's even stronger suggestion upthread: make it an error to specify a value requires rounding of any kind. In other words, if the minimum granularity is 1 minute, you can specify that as 60 seconds instead, but if you write 59 seconds, we error out. Maybe that seems pedantic, but I don't think users will much appreciate the discovery that 30 seconds means 60 seconds. They'll be happier to be told that up front than having to work it out afterward. I think this is totally wrong. The entire point of the GUC units system, or at least a large part of the point, is that users should not have to be intimately aware of the units in which a given value is measured internally. And that in turn means that the units had better be such that users won't find them overly coarse. If it matters a lot whether 59 seconds gets rounded to 60, then we didn't make a good choice of units for the GUC in question; and we should fix that choice, not mess with the rounding rules. The case where this argument falls down is for special values, such as where zero means something quite different from the smallest nonzero value. Peter suggested upthread that we should redefine any GUC values for which that is true, but (a) I think that loses on backwards compatibility grounds, and (b) ISTM zero is probably always special to some extent. A zero time delay for example is not likely to work. Maybe we should leave the rounding behavior alone (there's not much evidence that rounding in one direction is worse than another; although I'd also be okay with changing to round-to-nearest), and confine ourselves to throwing an error for the single case that an apparently nonzero input value is truncated/rounded to zero as a result of units conversion. 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] proposal: rounding up time value less than its unit.
Tom Lane-2 wrote Robert Haas lt; robertmhaas@ gt; writes: I liked David Johnston's even stronger suggestion upthread: make it an error to specify a value requires rounding of any kind. In other words, if the minimum granularity is 1 minute, you can specify that as 60 seconds instead, but if you write 59 seconds, we error out. Maybe that seems pedantic, but I don't think users will much appreciate the discovery that 30 seconds means 60 seconds. They'll be happier to be told that up front than having to work it out afterward. I think this is totally wrong. The entire point of the GUC units system, or at least a large part of the point, is that users should not have to be intimately aware of the units in which a given value is measured internally. And that in turn means that the units had better be such that users won't find them overly coarse. If it matters a lot whether 59 seconds gets rounded to 60, then we didn't make a good choice of units for the GUC in question; and we should fix that choice, not mess with the rounding rules. The case where this argument falls down is for special values, such as where zero means something quite different from the smallest nonzero value. Peter suggested upthread that we should redefine any GUC values for which that is true, but (a) I think that loses on backwards compatibility grounds, and (b) ISTM zero is probably always special to some extent. A zero time delay for example is not likely to work. Maybe we should leave the rounding behavior alone (there's not much evidence that rounding in one direction is worse than another; although I'd also be okay with changing to round-to-nearest), and confine ourselves to throwing an error for the single case that an apparently nonzero input value is truncated/rounded to zero as a result of units conversion. To Andres' point: SELECT unit, count(*) FROM pg_settings WHERE unit '' GROUP BY unit; (9.3 / Ubuntu) min (1 - log_rotation_age) s (10) ms (13) kb (7) 8kb (6) I don't know about the size implications but they seem to be non-existent. That any setting critically matters at +/- 1s or 1ms doesn't seem likely in practice. Even +/- 1min for a setting, if it did matter at extreme scale, would be recognizable by the user in practice as a rounding artifact and compensated for. At this point throwing an error for any precision that results in less than the default precision is my preference. I would not change the rounding rules for the simple reason that there is no obvious improvement to be had and so why introduce pointless change that - however marginal and unlikely - will be user-visible. The complaint to overcome is avoiding an interpretation of zero when the precision of the input is less than the GUC unit. Lacking any concrete complaints about our round-down policy I don't see where a change there is worthwhile. Fixing zero as a special value falls under the same category. As mathematically pure as using infinity may be the trade-off for practicality and usability seems, even in light of this complaint, like the correct one to have made. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816409.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] proposal: rounding up time value less than its unit.
Tomonari Katsumata t.katsumata1...@gmail.com writes: This patch rounds up the value when only it's less than required unit. .. Although my original complaint is fixed, I'm worried about this change will make users confusing. Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Is it better to raise a message(ex. INFO) when a value less than required unit is set? No. Non-error messages are probably completely useless in this area: users will typically never see such messages for settings made in postgresql.conf, because it will not occur to them to look in the postmaster log. So the behavior needs to be self-explanatory without any messages; and that means it had better not be very complicated. 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] proposal: rounding up time value less than its unit.
Tom Lane-2 wrote Tomonari Katsumata lt; t.katsumata1122@ gt; writes: This patch rounds up the value when only it's less than required unit. .. Although my original complaint is fixed, I'm worried about this change will make users confusing. Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Wasn't the goal that all non-zero values result in the feature being enabled? With round nearest there will still be some values that are non-zero but that round to zero and thus disable the feature. Values failing in the range (0, 1) in the canonical unit must be treated specially otherwise we might as well just leave the current behavior as-is since floor is likely just as good a rule as round-nearest. For fractions greater than one round nearest is probably fine and indeed on average results in the least amount of potential adjustment magnitude. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816007.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] proposal: rounding up time value less than its unit.
David G Johnston david.g.johns...@gmail.com writes: Tom Lane-2 wrote Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Wasn't the goal that all non-zero values result in the feature being enabled? With round nearest there will still be some values that are non-zero but that round to zero and thus disable the feature. Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or we could adopt Peter's idea that zero shouldn't be special (instead using, say, -1 to turn things off). But I'm afraid that would break way too many peoples' configuration choices. 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] proposal: rounding up time value less than its unit.
On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or make it an error to specify a value that rounds to 0 but isn't 0. -- greg -- 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] proposal: rounding up time value less than its unit.
Tom Lane-2 wrote David G Johnston lt; david.g.johnston@ gt; writes: Tom Lane-2 wrote Indeed. I have not understood why you are insisting on round up semantics. Wouldn't it make more sense for the behavior to be round to nearest? That would get rid of any worries about treating zero specially. Wasn't the goal that all non-zero values result in the feature being enabled? With round nearest there will still be some values that are non-zero but that round to zero and thus disable the feature. Ah. Okay, but then what's wrong with the original proposal of use ceil() instead of floor()? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or we could adopt Peter's idea that zero shouldn't be special (instead using, say, -1 to turn things off). But I'm afraid that would break way too many peoples' configuration choices. Yes, using ceil() is the most acceptable, for me, solution that does not involve throwing an error. Are there any examples of where treating zero specially is a problem or is this concern theoretical? We've already decided to risk enabling disabled features by applying this patch since the likelihood of someone relying on the rounding to keep the feature disabled (or at its default value in some instances) is unlikely. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816018.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] proposal: rounding up time value less than its unit.
Thank you for the comments. It was a bug in my patch as another developer says. I've not considered about the value 'zero', sorry. I attached new patch. This patch rounds up the value when only it's less than required unit. Like below. (unit: min) 0-0 0s-0 10s-1 70s-1 Although my original complaint is fixed, I'm worried about this change will make users confusing. Is it better to raise a message(ex. INFO) when a value less than required unit is set? 2014-08-21 21:00 GMT+09:00 Heikki Linnakangas hlinnakan...@vmware.com: On 07/10/2014 09:52 AM, Tomonari Katsumata wrote: Hi, Several couple weeks ago, I posted a mail to pgsql-doc. http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp In this thread, I concluded that it's better to round up the value which is less than its unit. Current behavior (rounding down) has undesirable setting risk, because some GUCs have special meaning for 0. And then I made a patch for this. Please check the attached patch. The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? If we're going to explain the rounding up in the manual, we also need to explain the normal rule, which is to round down. How about this: --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -44,6 +44,15 @@ (seconds), literalmin/literal (minutes), literalh/literal (hours), and literald/literal (days). Note that the multiplier for memory units is 1024, not 1000. + +para + If a memory or time setting is specified with more precision than the + implicit unit of the setting, it is rounded down. However, if rounding + down would yield a zero, it is rounded up to one instead. For example, + the implicit unit of varnamelog_rotation_age/varname is minutes, so if + you set it to literal150s/literal, it will be rounded down to two + minutes. However, if you set it to literal10s/literal, it will be + rounded up to one minute. /para - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers time-unit-guc-round-up_v2.patch Description: Binary data -- 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] proposal: rounding up time value less than its unit.
On 07/10/2014 09:52 AM, Tomonari Katsumata wrote: Hi, Several couple weeks ago, I posted a mail to pgsql-doc. http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp In this thread, I concluded that it's better to round up the value which is less than its unit. Current behavior (rounding down) has undesirable setting risk, because some GUCs have special meaning for 0. And then I made a patch for this. Please check the attached patch. The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? If we're going to explain the rounding up in the manual, we also need to explain the normal rule, which is to round down. How about this: --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -44,6 +44,15 @@ (seconds), literalmin/literal (minutes), literalh/literal (hours), and literald/literal (days). Note that the multiplier for memory units is 1024, not 1000. + +para + If a memory or time setting is specified with more precision than the + implicit unit of the setting, it is rounded down. However, if rounding + down would yield a zero, it is rounded up to one instead. For example, + the implicit unit of varnamelog_rotation_age/varname is minutes, so if + you set it to literal150s/literal, it will be rounded down to two + minutes. However, if you set it to literal10s/literal, it will be + rounded up to one minute. /para - Heikki -- 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] proposal: rounding up time value less than its unit.
Heikki Linnakangas hlinnakan...@vmware.com writes: The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? That sounds like a dealbreaker to me. There are enough places where zero has special meaning that we should not *ever* change zero to non-zero silently. 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] proposal: rounding up time value less than its unit.
On 8/21/14 11:16 AM, Tom Lane wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? That sounds like a dealbreaker to me. There are enough places where zero has special meaning that we should not *ever* change zero to non-zero silently. I don't think I like this idea anyway. If something has units of an hour and the user (perhaps misunderstanding the setting) sets it to one second, then we shouldn't silently change that to one hour. If there is a problem with rounding it to zero, then we should perhaps raise an error. (And stop treating zero specially. It's a terrible idea.) -- 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] proposal: rounding up time value less than its unit.
Peter Eisentraut-2 wrote On 8/21/14 11:16 AM, Tom Lane wrote: Heikki Linnakangas lt; hlinnakangas@ gt; writes: The patch also rounds a zero up to one. A naked zero with no unit is not affected, but e.g if you have log_rotation_age=0s, it will not disable the feature as you might expect, but set it to 1 minute. Should we do something about that? That sounds like a dealbreaker to me. There are enough places where zero has special meaning that we should not *ever* change zero to non-zero silently. I don't think I like this idea anyway. If something has units of an hour and the user (perhaps misunderstanding the setting) sets it to one second, then we shouldn't silently change that to one hour. If there is a problem with rounding it to zero, then we should perhaps raise an error. (And stop treating zero specially. It's a terrible idea.) I'm on board, from the original thread, for errors if the input cannot be converted to the parameter measurement unit cleanly. By which I mean the specified value should result in an integer being recorded without rounding. Specifying a precision less than the default unit thus becomes impossible. I don't have a problem with zero meaning disabled when appropriate since it avoids having a separate on/off GUC. That said the complaint here just seems like a bug in the supplied patch - zero is zero regardless of whether a unit is specified. The only obvious exception would be temperature but that isn't relevant here. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5815770.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] proposal: rounding up time value less than its unit.
Hi Robert, Thank you for checking this! I've added it to commitfest. https://commitfest.postgresql.org/action/patch_view?id=1507 regards, Tomonari Katsumata 2014-07-12 6:07 GMT+09:00 Robert Haas robertmh...@gmail.com: On Thu, Jul 10, 2014 at 2:52 AM, Tomonari Katsumata katsumata.tomon...@po.ntts.co.jp wrote: Several couple weeks ago, I posted a mail to pgsql-doc. http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp In this thread, I concluded that it's better to round up the value which is less than its unit. Current behavior (rounding down) has undesirable setting risk, because some GUCs have special meaning for 0. And then I made a patch for this. Please check the attached patch. Thanks for the patch. Please add it here: https://commitfest.postgresql.org/action/commitfest_view/open -- 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] proposal: rounding up time value less than its unit.
On Thu, Jul 10, 2014 at 2:52 AM, Tomonari Katsumata katsumata.tomon...@po.ntts.co.jp wrote: Several couple weeks ago, I posted a mail to pgsql-doc. http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp In this thread, I concluded that it's better to round up the value which is less than its unit. Current behavior (rounding down) has undesirable setting risk, because some GUCs have special meaning for 0. And then I made a patch for this. Please check the attached patch. Thanks for the patch. Please add it here: https://commitfest.postgresql.org/action/commitfest_view/open -- 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
[HACKERS] proposal: rounding up time value less than its unit.
Hi,
Several couple weeks ago, I posted a mail to pgsql-doc.
http://www.postgresql.org/message-id/53992ff8.2060...@po.ntts.co.jp
In this thread, I concluded that it's better to
round up the value which is less than its unit.
Current behavior (rounding down) has undesirable setting risk,
because some GUCs have special meaning for 0.
And then I made a patch for this.
Please check the attached patch.
regards,
---
Tomonari Katsumata
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 04ddd73..9aaffb0 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -43,7 +43,9 @@
are literalms/literal (milliseconds), literals/literal
(seconds), literalmin/literal (minutes), literalh/literal
(hours), and literald/literal (days). Note that the multiplier
- for memory units is 1024, not 1000.
+ for memory units is 1024, not 1000. And also note that if you set
+ less value than which is expected for the time setting, the value
+ would be rounded up.
/para
para
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3a31a75..8ba4879 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5122,9 +5122,11 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
switch (flags GUC_UNIT_TIME)
{
case GUC_UNIT_S:
+ val += (val MS_PER_S) ? MS_PER_S : 0;
val /= MS_PER_S;
break;
case GUC_UNIT_MIN:
+ val += (val MS_PER_MIN) ? MS_PER_MIN : 0;
val /= MS_PER_MIN;
break;
}
@@ -5138,6 +5140,7 @@ parse_int(const char *value, int *result, int flags, const char **hintmsg)
val *= MS_PER_S;
break;
case GUC_UNIT_MIN:
+ val += (val S_PER_MIN) ? S_PER_MIN : 0;
val /= S_PER_MIN;
break;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
