Re: [HACKERS] proposal: make NOTIFY list de-duplication optional
On Sat, 6 Feb 2016 at 12:50 Tom Lane wrote: > Robert Haas writes: > > I agree with what Merlin said about this: > > > http://www.postgresql.org/message-id/CAHyXU0yoHe8Qc=yc10ahu1nfia1tbhsg+35ds-oeueuapo7...@mail.gmail.com > > Yeah, I agree that a GUC for this is quite unappetizing. > How would you feel about a variant for calling NOTIFY? The SQL syntax could be something like "NOTIFY [ALL] channel, payload" where the ALL means "just send the notification already, nobody cares whether there's an identical one in the queue". Likewise we could introduce a three-argument form of pg_notify(text, text, bool) where the final argument is whether you are interested in removing duplicates. Optimising the remove-duplicates path is still probably a worthwhile endeavour, but if the user really doesn't care at all about duplication, it seems silly to force them to pay any performance price for a behaviour they didn't want, no? Cheers, BJ
Re: [HACKERS] psql: Activate pager only for height, not width
On Tue, 30 May 2017 at 05:30 Christoph Berg wrote: > Oh interesting, I didn't know about pager_min_lines. That sounds > useful as well. +1 on the analogous pager_min_cols option. > On closer inspection, I note that psql already has a 'columns' \pset option, which does control the width for triggering the pager, but also controls the width for wrapping and auto-expanded mode purposes. So, I can set 'columns' to get the pager behaviour that I want, but I also get unwanted effects where I'd rather let the default (terminal width) apply. So if we were to add a 'pager_min_cols', we'd have to decide how it interacts with 'columns'. For example, to determine whether to trigger the pager, we look for 'pager_min_cols' first, and if that is not set, then fall back to 'columns'. For all other width purposes, 'columns' would continue to apply as present. However, my feeling is that this is becoming a bit too fiddly. If 'columns' did not already exist then 'pager_min_cols' would make more sense, but as it does already exist, my preference is to leave 'columns' as-is, and go for a height-only option to 'pager' instead. Thoughts? Cheers, BJ
[HACKERS] psql: Activate pager only for height, not width
Hello hackers,
I am often frustrated by the default behaviour of the psql pager, which
will activate a pager if the output is deemed to be "too wide" for the
terminal, regardless of the number of lines output, and of the
pager_min_lines setting.
This behaviour is sometimes desirable, but in my use patterns it is more
often the case that I want the pager to activate for output longer than
terminal height, whereas for output a little wider than the terminal, I am
happy for there to be some wrapping. This is especially the case with "\d"
output for tables, where, at 80 columns, very often the only wrapping is in
the table borders and constraint/trigger definitions.
Usually I turn the pager off completely, and only switch it on when I am
about to execute something that will return many rows, but what I'd really
like is some way to tell psql to activate the pager as normal for height,
but to ignore width. My first thought was an alternate mode to \pset pager
-- to {'on' | 'off' | 'always'} we could add 'height'.
Another option is to add the ability to specify the number of columns which
psql considers "too wide", analogous to pager_min_lines. I could then set
pager_min_cols to something around 150 which would work nicely for my
situation.
I don't have strong opinions about how the options are constructed, as long
as it is possible to obtain the behaviour.
I would be happy to produce a patch, if this seems like an acceptable
feature add.
Regards,
BJ
Re: [HACKERS] psql: add \pset true/false
On Fri, 30 Oct 2015 at 00:51 Tom Lane wrote: > The really key argument that hasn't been addressed here is why does such > a behavior belong in psql, rather than elsewhere? Surely legibility > problems aren't unique to psql users. Moreover, there are exactly > parallel facilities for other datatypes on the server side: think > DateStyle or bytea_output. So if you were trying to follow precedent > rather than invent a kluge, you'd have submitted a patch to create a GUC > that changes the output of boolout(). > I find Tom's analogy to datestyle and bytea_output convincing. +1 for a GUC that changes the behaviour of boolout. And also +1 for doing anything at all to improve on the t/f output. Those glyphs are way too similar to each other. I think U+2713 and U+2717 (✓ and ✗) are the obvious choices for a boolean, but if we have a GUC we can set this to taste. Cheers, BJ
Re: [HACKERS] Future of our regular expression code
On 19 February 2012 06:52, Tom Lane wrote: > Yeah ... if you *don't* know the difference between a DFA and an NFA, > you're likely to find yourself in over your head. Having said that, > this is eminently learnable stuff and pretty self-contained, so somebody > who had the time and interest could make themselves into an expert in > a reasonable amount of time. I find myself in possession of both time and interest. I have to admit up-front that I don't have experience with regex code, but I do have some experience with parsers generally, and I'd like to think some of that skillset would transfer to this problem. I also find regexes fascinating and extremely useful, so learning more about them will be no hardship. I'd happily cede to an expert, should one appear, but otherwise I'm all for moving the regex code into a discrete library, and I'm volunteering to take a swing at it. Cheers, BJ -- 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] Future of our regular expression code
On 19 February 2012 15:49, Tom Lane wrote: > That sounds great. > > BTW, if you don't have it already, I'd highly recommend getting a copy > of Friedl's "Mastering Regular Expressions". It's aimed at users not > implementers, but there is a wealth of valuable context information in > there, as well as a really good not-too-technical overview of typical > implementation techniques for RE engines. You'd probably still want one > of the more academic presentations such as the dragon book for > reference, but I think Freidl's take on it is extremely useful. Thanks for the recommendations Tom. I've now got Friedl, and there's a dead-tree copy of 'Compilers' making its gradual way to me (no ebook). I've also been reading the article series by Russ Cox linked upthread -- it's good stuff. Are you far enough into the backrefs bug that you'd prefer to see it through, or would you like me to pick it up? Cheers, BJ -- 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] Future of our regular expression code
On 20 February 2012 10:42, Tom Lane wrote: > I have also got > a bunch of text about the colormap management code, which I think > is interesting right now because that is what we are going to have > to fix if we want decent performance for Unicode \w and related > classes (cf the other current -hackers thread about regexes). > I was hoping to prevail on you to pick that part up as your first > project. I will commit what I've got in a few minutes --- look > for src/backend/regex/README in that commit. Okay, I've read through your README content, it was very helpful. I'll now go chew through some more reading material and then start studying our existing regex source code. Once I'm firing on all cylinders with this stuff, I'll begin to tackle the colormap. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Our regex vs. POSIX on "longest match"
Hello folks, I am in the process of accelerating down the rabbit hole of regex internals. Something that came up during my reading, is that a POSIX compliant regex engine ought to always prefer the longest possible match, when multiple matches are possible beginning from the same location in the string. [1] I wasn't sure that that was how our regex engine worked, and indeed, on checking the manual [2] I found that our regex engine uses a strange sort of "inductive greediness" to determine whether the longest or the shortest possible match ought to be preferred. The greediness of individual particles in the regex are taken into account, and at the top level the entire expression is concluded to be either greedy, or non-greedy. I'll admit that this is a pretty obscure point, but we do appear to be in direct violation of POSIX here. I am getting the impression that our engine works this way to route around some of the performance issues that can arise in trying out every possible match with an NFA-style engine. I find it a bit unfortunate that POSIX is so unambiguous about this, while our engine's treatment is, well ... quite arcane by comparison. At minimum, I think we should be more explicit in the manual that this behaviour flips POSIX the proverbial bird. Several paragraphs south, there is a sentence reading "Incompatibilities of note include ... the longest/shortest-match (rather than first-match) matching semantics", but in the context it seems as though the author is talking about an incompatibility with Perl, not with POSIX. Thoughts? Cheers, BJ [1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html [2] http://www.postgresql.org/docs/9.0/static/functions-matching.html#FUNCTIONS-POSIX-REGEXP -- 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] Our regex vs. POSIX on "longest match"
On 4 March 2012 17:53, Tom Lane wrote: > Brendan Jurd writes: >> I'll admit that this is a pretty obscure point, but we do appear to be >> in direct violation of POSIX here. > > How so? POSIX doesn't contain any non-greedy constructs. If you use > only the POSIX-compatible greedy constructs, the behavior is compliant. While it's true that POSIX doesn't contemplate non-greed, after reading the spec I would have expected an expression *as a whole* to still prefer the longest match, insofar as that is possible while respecting non-greedy particles. I just ran some quick experiments in Perl, Python and PHP, using 'xy1234' ~ 'y*?\d+'. All returned 'y1234', which to my mind is the most obvious answer, and one which still makes sense in light of what POSIX has to say. Whereas Postgres (and presumably Tcl) return 'y1'. What I found surprising here is that our implementation allows an earlier quantifier to clobber the greediness of a later quantifier. There's a certain disregard for the intentions of the author of the regex. If I had wanted the whole expression to be non-greedy, I could have written 'y*?\d+?'. But since I wrote 'y*?\d+', it is clear that I meant for the first atom to be non-greedy, and for the second to be greedy. > The issue that is obscure is, once you define some non-greedy > constructs, how to define how they should act in combination with greedy > ones. I'm not sure to what extent the engine's behavior is driven by > implementation restrictions and to what extent it's really the sanest > behavior Henry could think of. I found a comment from him about it: > http://groups.google.com/group/comp.lang.tcl/msg/c493317cc0d10d50 > but it's short on details as to what alternatives he considered. Thanks for the link; it is always good to get more insight into Henry's approach. I'm beginning to think I should just start reading everything he ever posted to comp.lang.tcl ... Cheers, BJ -- 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] Our regex vs. POSIX on "longest match"
On 5 March 2012 04:34, Tom Lane wrote: > Brendan Jurd writes: >> On 4 March 2012 17:53, Tom Lane wrote: >>> Brendan Jurd writes: >> While it's true that POSIX doesn't contemplate non-greed, after >> reading the spec I would have expected an expression *as a whole* to >> still prefer the longest match, insofar as that is possible while >> respecting non-greedy particles. > > It's not apparent to me that a construct that is outside the spec > shouldn't be expected to change the overall behavior. In particular, > what if the RE contains only non-greedy quantifiers --- would you still > expect longest match overall? Henry certainly didn't. Well, no, but then that wouldn't be "prefer the longest match, insofar as that is possible while respecting non-greedy particles". If all the quantifiers in an expression are non-greedy, then it is trivially true that the only way to respect the author's intention is to return the shortest match. > Well, that's just an arbitrary example. The cases I remember people > complaining about in practice were the other way round: greedy > quantifier followed by non-greedy, and they were unhappy that the > non-greediness was effectively not respected (because the overall RE was > taken as greedy). I am unhappy about the reverse example too, and for the same reasons. If we look at 'xy1234' ~ 'y*\d+?', Postgres gives us 'y1234' (disregarding the non-greediness of the latter quantifier), and Python gives us 'y1' (respecting both quantifiers). So in Postgres, no matter how you mix the greediness up, some of your quantifiers will not be respected. > So you can't fix the issue by pointing to POSIX and > saying "overall greedy is always the right thing". "... insofar as that is possible while respecting non-greedy particles". I will take Henry's word for it that this problem is harder than it looks, but in any case, I think we may not presume to know better than the author of the regex about the greediness of his quantifiers. > Another thing I've seen people complain about is that an alternation > (anything with top-level "|") is always taken as greedy overall. Yeah, that is quirky. Cheers, BJ -- 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] Our regex vs. POSIX on "longest match"
On 5 March 2012 17:23, Robert Haas wrote: > This is different from what Perl does, but I think Perl's behavior > here is batty: given a+|a+b+ and the string aaabbb, it picks the first > branch and matches only aaa. Yeah, this is sometimes referred to as "ordered alternation", basically that the branches of the alternation are prioritised in the same order in which they are described. It is fairly commonplace among regex implementations. > apparently, it selects the syntactically first > branch that can match, regardless of the length of the match, which > strikes me as nearly pure evil. As long as it's documented that alternation prioritises in this way, I don't feel upset about it. At least it still provides you with a sensible way to get whatever you want from your RE; if you want a shorter alternative to be preferred, put it up the front. Ordered alternation also gives you a way to specify which of several same-length alternatives you would prefer to be matched, which can come in handy. It also means you can specify less-complex alternatives before more-complex ones, which can have performance advantages. I do agree with you that if you *don't* do ordered alternation, then it is right to treat alternation as greedy by default. Cheers, BJ -- 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] 9.3: Empty arrays returned by array_remove()
On 31 May 2013 02:52, Dean Rasheed wrote: > Testing 9.3beta, it seems that array_remove() may return an empty 1-d > array whose upper bound is lower than its lower bound. I know that we > discussed allowing this kind of array, but I don't think that > discussion reached any conclusion, other than to agree that the > current empty 0-d array behaviour would be kept in 9.3. > That's right, zero-D is still the only supported representation of an empty array, so when array_remove() yields an empty array it ought to be zero-D. Good catch. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)
On 12 June 2013 04:43, Josh Berkus wrote: > What's the status on this patch and current approach to ZDA? Alright, it might be a good idea to have a quick recap. Last time, on Arrays Of Our Lives ... So I proposed and posted a patch aimed at deprecating zero-D arrays, and replacing them with 1-D empty arrays. That kicked off a long discussion, with some folks (Robert, Merlin, Florian et al) vigorously opposing any change in the behaviour of the array_(length|ndims|lower|upper) functions. To those folks, the behaviour would have to be a lot worse than it currently is to justify breaking compatibility with existing applications. The idea of using a GUC to smooth over the compatibility break was suggested, and firmly rejected. The idea of creating an entirely new type with nicer behaviours came up, but that wouldn't really fly because arrays have already hogged all the best syntax. Since compatibility breakage is so contentious, I suggested that we forget about changing the array representation and just add new functions with more sensible behaviours: * cardinality(anyarray) to return the length of the first dimension, zero if empty, and * array_num_items(anyarray) to return the total number of element positions per ArrayGetNItems, zero if empty. There have been attempts to add a cardinality function in the past, as it is required by the SQL spec, but these attempts have stalled when trying to decide how it should handle multidim arrays. Having it return the length of the first dimension is the more spec-compatible way to go, but some folks argued that it should work as ArrayGetNItems, because we don't already have a function for that at the SQL level. Therefore I propose we add cardinality() per the spec, and another function to expose ArrayGetNItems. And that's about where we got to, when the whole discussion was put on a time-out to make room for the beta. I am withdrawing the original zero-D patch in favour of the proposed new functions. If you have an opinion about that, please do chime in. Depending on how that goes I may post a patch implementing my new proposal in the next few days. Cheers, BJ -- 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] [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)
On 12 June 2013 18:22, Dean Rasheed wrote: > +1 for having a function to return the total number of elements in an > array, because that's something that's currently missing from SQL. > > However, I think that CARDINALITY() should be that function. > > I'm not convinced that having CARDINALITY() return the length of the > first dimension is more spec-compatible, since our multi-dimensional > arrays aren't nested arrays, and it seems unlikely that they ever will > be. I'd argue that it's at least equally spec-compatible to have > CARDINALITY() return the total number of elements in the array, if you > think of a multi-dimensional array as a collection of elements > arranged in a regular pattern. It's true that our multidims aren't nested, but they are the nearest thing we have. If we want to keep the door open for future attempts to nudge multidim arrays into closer approximation of nested arrays, it would be better to have the nested interpretation of CARDINALITY. Given what we've just gone through with array_length, it seems that once we select a behaviour for CARDINALITY, we will be stuck with it permanently. The problem with thinking of our multidim arrays as just a weirdly crumpled arrangement of a single collection, is that we've already abused the nesting syntax for declaring them. > Also, the spec describes CARDINALITY() and UNNEST() using the same > language, and I think it's implicit in a couple of places that > CARDINALITY() should match the number of rows returned by UNNEST(), > which we've already implemented as fully unnesting every element. > > We're about to add ORDINALITY to UNNEST(), and to me it would be very > odd to have the resulting maximum ordinality exceed the array's > cardinality. Yeah, that makes sense. Well the good news is that either way, CARDINALITY will do what people want in the most common case where the array is one-dimensional. Multidim arrays are why we can't have nice things. Cheers, BJ -- 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] [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)
On 13 June 2013 04:26, Merlin Moncure wrote:
> On Wed, Jun 12, 2013 at 1:20 PM, Tom Lane wrote:
>> Josh Berkus writes:
>>> On 06/12/2013 11:01 AM, Tom Lane wrote:
I'm going to be disappointed if all we can get out of this is
a cardinality() function, and nothing is done about the empty-array
semantics.
I would be disappointed too, but on the other hand, CARDINALITY is
required by the spec and anything would be better than nothing.
>> Meh. Robert was pretty vocal about it, but it wasn't clear to me that
>> his was the majority opinion, and in any case there wasn't much
>> consideration given to compromises falling somewhere between "no
>> changes" and the rather drastic solution Brendan proposed.
I'm all for looking into possible compromises, and will happily take
any improvements to this mess I think I can get past the compatibility
maximalist caucus.
>> regression=# select array_dims('{}'::int[]) is null;
>> ?column?
>> --
>> t
>> (1 row)
>>
>> Whatever you think the dimensions of that are, surely they're not
>> unknown.
I don't think anyone has actually tried to defend the behaviour of the
array functions w.r.t. empty arrays. Even the opponents of the
original proposal agreed that the behaviour was silly, they just
didn't want to fix it, on account of the upgrade burden.
> But, couldn't that be solved by deprecating that function and
> providing a more sensible alternatively named version?
And what would you name that function? array_dims2? I can't think of
a name that makes the difference in behaviour apparent. Can you
imagine the documentation for that?
array_dims - Returns the dimensions of the array, unless it is empty
in which case NULL.
array_proper_dims - Returns the dimensions of the array.
array_ndims - Returns the number of dimension, unless it is empty in
which case NULL.
array_proper_ndims - Returns the number of dimensions.
... and so on for _length, _upper and _lower.
Cheers,
BJ
--
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] [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)
On 14 June 2013 03:53, David E. Wheeler wrote: > Similar things should have dissimilar names. I propose: > > > > Old |New > --+-- > array_dims | array_desc array_bounds? > array_ndims | array_depth > array_length | array_size > array_lower | array_start > array_upper | array_finish > > The last two are meh, but it’s a place to start… I think that even with the most dissimilar names we can come up with, this is going to confuse people. But it is still better than doing nothing. I wonder whether, if we go in this direction, we could still use some of the work I did on deprecating zero-D arrays. Let's say the old functions keep doing what they do now, and we teach them to treat all empty arrays the same way they currently treat zero-D arrays (return NULL). The new functions treat zero-D arrays as though they were 1-D empty with default bounds, and we add CARDINALITY per ArrayGetNItems. This way, applications would not be broken by upgrading, and we'd be giving people a way to opt-in to a better API. Cheers, BJ -- 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] Hard to Use WAS: Hard limit on WAL space
On 15 June 2013 14:43, Craig Ringer wrote: > The #1 question I see on Stack Overflow has to be confusion about > pg_hba.conf, mostly from people who have no idea it exists, don't understand > how to configure it, etc. The totally non-obvious name of the file probably has something to do with that. It should be called 'auth.conf'. Cheers, BJ -- 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] Hard to Use WAS: Hard limit on WAL space
On 15 June 2013 16:18, Craig Ringer wrote: > On 06/15/2013 02:08 PM, Brendan Jurd wrote: >> On 15 June 2013 14:43, Craig Ringer wrote: >>> The #1 question I see on Stack Overflow has to be confusion about >>> pg_hba.conf, mostly from people who have no idea it exists, don't understand >>> how to configure it, etc. >> The totally non-obvious name of the file probably has something to do >> with that. It should be called 'auth.conf'. > Not convinced; since it only controls one facet of auth - it doesn't > define users, passwords, grants, etc ... When somebody is setting up postgres for the first time, and they list the contents of the config directory, you want them to have some idea what each of the files is for. If they see something called 'auth.conf', they'll get the right general idea. An understanding of the nuances (like that it doesn't control user accounts) will come once they open up the file -- which they may well do, because it is called 'auth.conf', and 'auth' is a thing you want to configure. If they see something called 'pg_hba.conf', they may very reasonably assume that it is some internal/advanced stuff that they don't need to worry about just yet, because what the heck is a 'pg_hba'? The 'pg' is unnecessary and the 'hba' is an internal jargon term that we've ill-advisedly allowed to leak out into the filename. If you really feel that 'auth.conf' is too imprecise, maybe something like 'conn-auth.conf' would be more your style. Cheers, BJ -- 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] [9.4 CF 1] The Commitfest Slacker List
On 25 June 2013 04:13, Joshua D. Drake wrote: > On 06/24/2013 10:59 AM, Andres Freund wrote: >> On 2013-06-24 10:50:42 -0700, Josh Berkus wrote: >>> This project is enormously stingy with giving credit to people. It's >>> not like it costs us money, you know. >> I am all for introducing a "Contributed by reviewing: ..." section in >> the release notes. > > It should be listed with the specific feature. I don't have a strong opinion about whether the reviewers ought to be listed all together or with each feature, but I do feel very strongly that they should be given some kind of credit. Reviewing is often not all that much fun (compared with authoring) and as Josh points out, giving proper credit has a "bang for buck" incentive value that is hard to argue with. Cheers, BJ -- 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] Kudos for Reviewers -- straw poll
On 26 June 2013 03:17, Josh Berkus wrote: > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch A weak preference for (c), with (b) running a close second. As others have suggested, a review that leads to significant commitable changes to the patch should bump the credit to co-authorship. > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count (b), the review should at least look at usabililty, doc, and regression test criteria even if there is no in-depth code analysis. > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too Provisionally (b), if we first try giving proper credit, and that still doesn't drum up enough reviewing, then look to further incentive schemes. No need to jump the gun. Cheers, BJ -- 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: simple date constructor from numeric values
On 1 July 2013 17:47, Pavel Stehule wrote: > 2013/6/29 Pavel Stehule : >> long time I am thinking about simple function for creating date or >> timestamp values based on numeric types without necessity to create >> format string. >> >> What do you think about this idea? > I found so same idea was discussed three years ago > > http://www.postgresql.org/message-id/14107.1276443...@sss.pgh.pa.us > I suggested something similar also: http://www.postgresql.org/message-id/AANLkTi=W1wtcL7qR4PuQaQ=uoabmjsusz6qgjtucx...@mail.gmail.com The thread I linked died off without reaching a consensus about what the functions ought to be named, although Josh and Robert were generally supportive of the idea. The function signatures I have been using in my own C functions are: * date(year int, month int, day int) returns date * datetime(year int, month int, day int, hour int, minute int, second int) returns timestamp Cheers, BJ -- 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: simple date constructor from numeric values
On 3 July 2013 21:41, Pavel Stehule wrote: > I am thinking so for these functions exists some consensus - minimally > for function "date"(year, month, int) - I dream about this function > ten years :) > > I am not sure about "datetime": > a) we use "timestamp" name for same thing in pg > b) we can simply construct timestamp as sum of date + time, what is > little bit more practical (for me), because it doesn't use too wide > parameter list. I agree. I've got no issues with using date + time arithmetic to build a timestamp. > what do you think about names? > > make_date > make_time I am fine with those names. 'make', 'construct', 'build', etc. are all reasonable verbs for what the functions do, but 'make' is nice and short, and will be familiar to people who've used a 'mktime'. > I don't would to use to_date, to_time functions, a) because these > functions use formatted input, b) we hold some compatibility with > Oracle. Yes, I agree. Cheers, BJ -- 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] operator dependency of commutator and negator, redux
On 20 December 2012 11:51, Tom Lane wrote: > > While reconsidering the various not-too-satisfactory fixes we thought of > back then, I had a sudden thought. Instead of having a COMMUTATOR or > NEGATOR forward reference create a "shell" operator and link to it, > why not simply *ignore* such references? Then when the second operator > is defined, go ahead and fill in both links? Ignore with warning sounds pretty good. So it would go something like this? # CREATE OPERATOR < (... COMMUTATOR >); WARNING: COMMUTATOR > (foo, foo) undefined, ignoring. CREATE OPERATOR # CREATE OPERATOR > (... COMMUTATOR <); CREATE OPERATOR Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Function to get size of notification queue?
Hi folks, I have a project which uses Postgres asynchronous notifications pretty heavily. It has a particularly Fun failure mode which causes the notification queue to fill up. To better debug this problem I'd like to be able to monitor the size of the notification queue over time. It doesn't look like we have a pg_notify_queue_size() or equivalent. Should we? Or would I be better off just watching the size of the pg_notify/ directory on disk? Cheers, BJ -- 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] Considering Gerrit for CFs
On 7 February 2013 08:07, Josh Berkus wrote: > The existing Gerrit community would be keen to have the PostgreSQL > project as a major user, though, and would theoretically help with > modification needs. Current major users are OpenStack, Mediawiki, > LibreOffice and QT. Do we actually have any requirements that are known to be uncatered for in gerrit's standard feature set? Cheers, BJ -- 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] Should array_length() Return NULL
On 16 March 2013 09:07, Tom Lane wrote:
> "David E. Wheeler" writes:
>> This surprised me:
>
>> david=# select array_length('{}'::text[], 1);
>> array_length
>> --
>>[null]
>
>> I had expecte dit to retur 0. I might expect NULL for a NULL param, but not
>> one that's defined but has no elements.
>
> The thing is that that syntax creates an array of zero dimensions,
> not one that has 1 dimension and zero elements. So "0" would be
> incorrect.
>
I'm going to ask the question that immediately comes to mind: Is there
anything good at all about being able to define a zero-dimensional
array?
I would have thought that anything deserving the name "array" has
one-or-more dimensions, and that a "zero-dimensional array" is a weird
way of talking about a scalar value. In which case '{}'::text[] would
not be a legitimate way to declare one anyway. Am I missing
something?
Cheers,
BJ
--
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] Should array_length() Return NULL
On 17 March 2013 05:19, Tom Lane wrote:
> Brendan Jurd writes:
>> On 16 March 2013 09:07, Tom Lane wrote:
>>> The thing is that that syntax creates an array of zero dimensions,
>>> not one that has 1 dimension and zero elements.
>
>> I'm going to ask the question that immediately comes to mind: Is there
>> anything good at all about being able to define a zero-dimensional
>> array?
>
> Perhaps not. I think for most uses, a 1-D zero-length array would be
> just as good. I guess what I'd want to know is whether we also need
> to support higher-dimensional zero-size arrays, and if so, what does
> the I/O syntax for those look like?
If I'm reading right, in our current implementation of array
dimensionality, there can be no such thing as a higher-dimensional
zero-length array anyhow. Postgres doesn't care about how many
dimensions you define for an array, it uses the "quacks like a duck"
test for number of dimensions. For example:
postgres=# SELECT ARRAY[1]::int[][], array_dims(ARRAY[1]::int[][]);
array | array_dims
---+
{1} | [1:1]
postgres=# SELECT ARRAY[ARRAY[1]]::int[];
array
---
{{1}}
postgres=# SELECT ARRAY[ARRAY[1]]::int[][];
array
---
{{1}}
Some array functions just plain don't work with multiple dimensions:
postgres=# SELECT array_append(ARRAY[ARRAY[1]]::int[][], ARRAY[2]);
ERROR: function array_append(integer[], integer[]) does not exist
So, to answer your question, no, I don't think we would need to
support it, at least not unless/until there is a major change and
number of dimensions becomes more meaningful. You can start out with
a zero-length array (which has one dimension) and then add nested
arrays to it if you want to -- it will then ipso facto have multiple
dimensions.
> Another fly in the ointment is that if we do redefine '{}' as meaning
> something other than a zero-D array, how will we handle existing
> database entries that are zero-D arrays?
>
I would go with zero-length 1-D. It's almost certainly what the
author intended.
I'd be more worried about the possibility of, say, PL/pg functions in
the field that rely on our existing bizarre behaviours to test for an
"empty" array, like IF array_length(A) IS NULL, or IF array_dims(A) IS
NULL. I'm pretty sure I have some such tests in my applications, and
I still think breaking them is a reasonable price to pay for greater
sanity.
Cheers,
BJ
--
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] Should array_length() Return NULL
On 17 March 2013 06:27, Tom Lane wrote: > What I'm concerned about here is whether these expressions shouldn't > be yielding different data values: > > > Right now, if we did make them produce what they appear to mean, the > array I/O functions would have a problem with representing the results: > > So I think we'd need to fix that before we could go very far in this > direction. I agree. I am starting to work on that very thing. I noticed that there are a whole bunch of errmsgs in ArrayCount and ReadArrayStr that just say "malformed array literal" with no detail message at all. Not very helpful. I'm tempted to improve that on my way past. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 17 March 2013 05:19, Tom Lane wrote:
> Brendan Jurd writes:
>> On 16 March 2013 09:07, Tom Lane wrote:
>>> The thing is that that syntax creates an array of zero dimensions,
>>> not one that has 1 dimension and zero elements.
>
>> I'm going to ask the question that immediately comes to mind: Is there
>> anything good at all about being able to define a zero-dimensional
>> array?
>
> Perhaps not. I think for most uses, a 1-D zero-length array would be
> just as good. I guess what I'd want to know is whether we also need
> to support higher-dimensional zero-size arrays, and if so, what does
> the I/O syntax for those look like?
>
> Another fly in the ointment is that if we do redefine '{}' as meaning
> something other than a zero-D array, how will we handle existing
> database entries that are zero-D arrays?
>
Hello hackers,
I submit a patch to rectify the weird and confusing quirk of Postgres
to use "zero dimensions" to signify an empty array.
Rationale:
The concept of an array without dimensions is a) incoherent, and b)
leads to astonishing results from our suite of user-facing array
functions. array_length, array_dims, array_ndims, array_lower and
array_upper all return NULL when presented such an array.
When a user writes ARRAY[]::int[], they expect to get an empty array,
but instead we've been giving them a bizarre semi-functional
proto-array. Not cool.
Approach:
The patch teaches postgres to regard zero as an invalid number of
dimensions (which it very much is), and allows instead for fully armed
and operational empty arrays.
The simplest form of empty array is the 1-D empty array (ARRAY[] or
'{}') but the patch also allows for empty arrays over multiple
dimensions, meaning that the final dimension has a length of zero, but
the other dimensions may have any valid length. For example,
'{{},{},{}}' is a 2-D empty array with dimension lengths {3,0} and
dimension bounds [1:3][1:0].
An empty array dimension may have any valid lower bound, but by
default the lower bound will be 1 and the upper bound will therefore
be 0.
Any zero-D arrays read in from existing database entries will be
output as 1-D empty arrays from array_recv.
There is an existing limitation where you cannot extend a multi-D
array by assigning values to subscripts or slices. I've made no
attempt to address that limitation.
The patch improves some error reporting, particularly by adding
errdetail messages for ereports of problems parsing a curly-brace
array literal.
There is some miscellaneous code cleanup included; I moved the
hardcoded characters '{', '}', etc. into constants, unwound a
superfluous inner loop from ArrayCount, and corrected some mistakes in
code comments and error texts. If preferred for reviewing, I can
split those changes (and/or the new errdetails) out into a separate
patch.
Incompatibility:
This patch introduces an incompatible change in the behaviour of the
aforementioned array functions -- instead of returning NULL for empty
arrays they return meaningful values. Applications relying on the old
behaviour to test for emptiness may be disrupted. One can
future-proof against this by changing e.g.
IF array_length(arr, 1) IS NULL ...
to
IF coalesce(array_length(arr, 1), 0) = 0 ...
There is also a change in the way array subscript assignment works.
Previously it wasn't possible to make a distinction between assigning
to an empty array and assigning to a NULL, so in either case an
expression like "arr[4] := 1" would create "[4:4]={1}". Now there is
a distinction. If you assign to an empty array you will get "{NULL,
NULL, NULL, 1}", whereas if you assign to a NULL you'll get
"[4:4]={1}".
Regression tests:
The regression tests were updated to reflect behavioural changes.
Documentation:
A minor update to the prose in chapter 8.15 is included in the patch.
Issues:
Fixed-length arrays (like oidvector) are zero-based, which means that
they are rendered into string form with their dimension info shown.
So an empty oidvector now renders as "[0:-1]={}", which is correct but
ugly. I'm not delighted with this side-effect but I'm not sure what
can be done about it. I'm open to ideas.
Diffstat:
doc/src/sgml/array.sgml | 4 +-
src/backend/executor/execQual.c | 77 +-
src/backend/utils/adt/array_userfuncs.c | 23 +-
src/backend/utils/adt/arrayfuncs.c | 671
+--
src/backend/utils/adt/arrayutils.c | 4 +
src/backend/utils/adt/xml.c | 19 +-
src/include/c.h | 1 +
src/include/utils/array.h | 4 +
src/pl/plpython/plpy_typeio.c | 3 -
src/test/isolatio
[HACKERS] Single-argument variant for array_length and friends?
Hi hackers, While I was working on my empty array patch I was frequently irritated by the absence of an array_length(anyarray). The same goes for array_upper and array_lower. Most of the time when I work with arrays, they are 1-D, and it's inelegant to having to specify which dimension I mean when there is only one to choose from. The question I have (and would appreciate your input on) is how such single-argument variants should behave when operating on an array with multiple dimensions? I see a few different options: a) throw an error like "Dimension argument is required for multiple dimension arrays" b) assume the user means the final (innermost) dimension c) assume the user means the first (outermost) dimension My inclination is towards (c) but I don't use multi-D arrays in postgres in the field. Opinions? Cheers, BJ -- 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] Single-argument variant for array_length and friends?
On 21 March 2013 17:08, Pavel Stehule wrote: > 2013/3/21 Tom Lane : >> I'm not entirely convinced that this is a good idea, but if we're going >> to allow it I would argue that "array_length(a)" should be defined as >> "array_length(a, 1)". The other possibilities are too complicated to >> explain in as few words. >> > > exactly > > +1 Hi Pavel, Is your +1 to array_length(a) being defined as array_length(a,1), or to Tom's being unconvinced by the whole proposal? Or both? Cheers, BJ -- 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] Single-argument variant for array_length and friends?
On 21 March 2013 17:32, Pavel Stehule wrote: > If I though about it more, I like to more limit one parametric > array_length function just for only 1D array. So it is your A use > case. But I understand so this variant is not orthogonal. Hard to say, > what is better. > Yes, for me (a) is running a very close 2nd place to (c). The strength of (a) is it means we aren't making guesses about the user's intention. When a user concocts an expression that is ambiguous, I feel it is usually good to kick it back to them and ask them to be more precise. On the other hand, I find it very natural to interpret "what is the length of my multidim array" to mean "what is the length of the outermost dimension of my multidim array", because to me a multidim array is just an array that contains more arrays. Cheers, BJ -- 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] Single-argument variant for array_length and friends?
On 22 March 2013 09:12, Merlin Moncure wrote: > On Thu, Mar 21, 2013 at 2:00 AM, Pavel Stehule > wrote: >> lot of postgresql functions calculate with all items in array without >> respect to dimensions - like unnest. >> >> so concept "use outermost dim" is not in pg now, and should not be >> introduced if it is possible. More it goes against a verbosity concept >> introduced by ADA and reused in PL/SQL and PL/pgSQL. > > and pl/psm* Yeah, okay. That argument works for me. Let's go for option (a), only allow the user to omit the dimension argument if the array is 1-D. We still have the issue that Tom isn't convinced that the feature is worth pursuing -- Tom, would you please elaborate a little on what you dislike about it? I don't see much of a downside (just 3 extra pg_procs). Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 25 March 2013 13:02, Josh Berkus wrote: > On 03/20/2013 04:45 PM, Brendan Jurd wrote: >> Incompatibility: >> This patch introduces an incompatible change in the behaviour of the >> aforementioned array functions -- instead of returning NULL for empty >> arrays they return meaningful values. Applications relying on the old >> behaviour to test for emptiness may be disrupted. One can > > As a heavy user of arrays, I support this patch as being worth the > breakage of backwards compatibility. However, that means it certainly > will need to wait for 9.4 to provide adequate warning. Thanks Josh, I appreciate the support. > > Brendan, how hard would it be to create a GUC for backwards-compatible > behavior? Good idea. I don't know how hard it would be (never messed with GUCs before), but I'll take a swing at it and see how it works out. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 26 March 2013 00:30, Tom Lane wrote: > Brendan Jurd writes: >> On 25 March 2013 13:02, Josh Berkus wrote: >>> Brendan, how hard would it be to create a GUC for backwards-compatible >>> behavior? > >> Good idea. > > No, it *isn't* a good idea. GUCs that change application-visible > semantics are dangerous. We should have learned this lesson by now. > They are? Well okay then. I'm not deeply attached to the GUC thing, it just seemed like a friendly way to ease the upgrade path. But if it's dangerous somehow I'm happy to drop it. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 21 March 2013 10:45, Brendan Jurd wrote: > src/test/isolation/expected/timeouts.out| 16 +- > src/test/isolation/specs/timeouts.spec | 8 +- Oops, looks like some unrelated changes made their way into the original patch. Apologies. Here's a -v2 patch, sans stowaways. Cheers, BJ zero-length-array-v2.diff.bz2 Description: BZip2 compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)
On 26 March 2013 05:26, Greg Stark wrote: > I'm not as sanguine as Tom is about how likely these corner cases will > be met actually. As far as I can tell checking IS NULL on > array_length() was the supported way to check for 0-length arrays > previously Correct. There was no other way to check for empty arrays, because every empty array was zero-D, and zero-D returns NULL from all array interrogation functions, even array_ndims (which is totally bonkers). > > At least if it's a clean break then everyone on 9.3 knows they need to > do IS NULL and everyone on 9.4 knows they need to check for 0. And, as I pointed out in at the top of the thread, you can write "coalesce(array_length(a,1), 0) = 0" if you want to be confident your code will continue work in either case, and for some folks I expect that will be the least painful way to upgrade from 9.3 -> 9.4. Cheers, BJ -- 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] adding support for zero-attribute unique/etc keys
On 26 March 2013 05:04, Darren Duncan wrote: > On 2013.03.25 1:17 AM, Albe Laurenz wrote: >> The desired effect can be had today with a unique index: >> >> CREATE TABLE singleton (id integer); >> CREATE UNIQUE INDEX singleton_idx ON singleton((1)); > > Okay, that is helpful, and less of a kludge than what I was doing, but it is > still a kludge compared to what I'm proposing, which I see as elegant. > FWIW I think an index on (TRUE) expresses the intention more clearly than an index on () would. I don't have any objection to the purely logical sense of the zero-attribute key, but it's hard to see the pragmatic value. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 26 March 2013 22:57, Robert Haas wrote:
> They hate it twice as much when the change is essentially cosmetic.
> There's no functional problems with arrays as they exist today that
> this change would solve.
>
We can't sensibly test for whether an array is empty. I'd call that a
functional problem.
The NULL return from array_{length,lower,upper,ndims} is those
functions' way of saying their arguments failed a sanity check. So we
cannot distinguish in a disciplined way between a valid, empty array,
and bad arguments. If the zero-D implementation had been more
polished and say, array_ndims returned zero, we had provided an
array_empty function, or the existing functions threw errors for silly
arguments instead of returning NULL, then I'd be more inclined to see
your point. But as it stands, the zero-D implementation has always
been half-baked and slightly broken, we just got used to working
around it.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 27 March 2013 06:47, Robert Haas wrote:
> On Tue, Mar 26, 2013 at 9:02 AM, Brendan Jurd wrote:
>> We can't sensibly test for whether an array is empty. I'd call that a
>> functional problem.
>
> Sure you can. Equality comparisons work just fine.
>
> rhaas=# select '{}'::int4[] = '{}'::int4[];
The good news is, if anybody out there is using that idiom to test for
emptiness, they will not be disrupted by the change.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 28 March 2013 00:21, Dean Rasheed wrote:
> The patch is also allowing '{{},{},{}}' which is described up-thread
> as a 2-D empty array. That's pretty misleading, since it has length 3
> (in the first dimension). '{{},{}}' and '{{}}' are both "more empty",
> but neither is completely empty.
> I'm not saying that the current situation is not broken. I'm just
> questioning whether the fix is actually any less confusing than what
> we have now.
Well the fix is primarily about 1-D empty arrays, and in that respect
it is much less confusing than what we have now. As for multidim
arrays, I don't use them in the field, so I don't have a strong
opinion about how (or even whether) we should support empty multidim.
I included the '{{}}' syntax following comments from Tom that we
should consider supporting that if we were to get rid of zero-D, but I
don't really have any skin in that game.
Since we require multidim arrays to be regular, perhaps they would
need extents in all dimensions to be practically useful ... if you
wanted to define a blank tic-tac-toe board using a 2-D array, for
example, you'd probably define it as 3x3 with NULL values in each
position, rather than trying to make it "empty".
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 28 March 2013 09:39, Dean Rasheed wrote:
> On 27 March 2013 17:14, Brendan Jurd wrote:
>> Well the fix is primarily about 1-D empty arrays, and in that respect
>> it is much less confusing than what we have now.
>
> Maybe. But even in 1-D, it's still jumping from having one empty array
> to infinitely many starting at different indexes, e.g., '{}'::int[] !=
> '[4:3]={}'::int[]. There may be a certain logic to that, but I'm not
> convinced about its usefulness.
We already have the ability to define lower bounds other than 1 on
arrays, and it would be inconsistent to allow that for arrays with
elements, but not for arrays without. I could imagine somebody
wanting to create an empty zero-based array, and then iteratively
append elements to it.
> Also, it is incompatible with the choice made for empty ranges,
To me it doesn't make sense to try to draw parallels between arrays
and ranges, they really are completely different things.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 28 March 2013 20:34, Dean Rasheed wrote:
> Is the patch also going to allow empty arrays in higher dimensions
> where not just the last dimension is empty?
It doesn't allow that at present.
> It seems as though, if
> it's allowing 1-by-0 arrays like '{{}}' and '[4:4][8:7]={{}}', it
> should also allow 0-by-0 arrays like '[4:3][8:7]={}', and 0-by-3
> arrays like '[4:3][11:13]={}'.
I think the array literal parser would reject this on the grounds that
the brace structure doesn't match the dimension specifiers. You could
modify that check to respect zero length in dimensions other than the
innermost one, but it's hard to say whether it would be worth the
effort.
It might be instructive to hear from somebody who does (or intends to)
use multidim arrays for some practical purpose, but I don't even know
whether such a person exists within earshot of this list.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 1 April 2013 21:57, Robert Haas wrote:
> On Tue, Mar 26, 2013 at 4:39 PM, Brendan Jurd wrote:
>> On 27 March 2013 06:47, Robert Haas wrote:
>>> rhaas=# select '{}'::int4[] = '{}'::int4[];
>>
>> The good news is, if anybody out there is using that idiom to test for
>> emptiness, they will not be disrupted by the change.
>
> According to the discussion downthread, apparently they will, because
> you're introducing an infinitude of empty arrays, not all of which
> compare equal to '{}'::int4.
It is not possible to construct e.g. '[3:2]={}' or '{{}, {}}' in
existing applications, so there is no way for that idiom in existing
applications to be broken by upgrading. If testing for equality with
'{}' works now, it will also work post-upgrade.
The only way for it to stop working is if somebody upgrades, and
*then* goes out of their way to create an empty array with nondefault
lower bounds, and then tries to compare that array against the empty
array with default lower bounds, to test for emptiness. Which would
be silly.
Big picture: A very large number of users wouldn't be using arrays at
all, and of those who are, probably a vanishingly small number
(perhaps zero) care about how emptiness interacts with multiple
dimensions or nondefault lower bounds. We're talking about a corner
case inside a corner case here.
For most folks, this upgrade would break nothing. A few (myself
included) will want to grep their code for
array_(lower|upper|length|dims) call sites and maybe make some tweaks.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 2 April 2013 10:59, Robert Haas wrote:
> On Mon, Apr 1, 2013 at 6:40 PM, Brendan Jurd wrote:
>> It is not possible to construct e.g. '[3:2]={}' or '{{}, {}}' in
>> existing applications, so there is no way for that idiom in existing
>> applications to be broken by upgrading. If testing for equality with
>> '{}' works now, it will also work post-upgrade.
>
> Sure, they've probably got to have at least
> some kind of application change before the wheels really start to come
> off, but as soon as some array that's empty but not equal to {} creeps
> into their application by whatever means, they've got trouble.
Constructing an array with nondefault bounds isn't happening by
accident, you'd have to know about, and deliberately make use of, the
obscure '[m:n]={}' syntax for array literals. How is it going to
"creep in"?
I note the total absence of people rending their garments over the
fact that '{foo}' does not equal '[2:2]={foo}'.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 2 April 2013 11:34, David E. Wheeler wrote: > On Apr 1, 2013, at 4:59 PM, Robert Haas wrote: > >> I think the only people for whom nothing will break are the people who >> aren't using arrays in the first place. Anyone who is is likely to >> have dependencies on the way array_lower/upper work today. > > Well, what if we add new functions that return 0 for empty arrays, but leave > the existing ones alone? Perhaps call them array_size(), array_first_index(), > and array_last_index(). Then nothing has to break, and we can decide > independently if we want to deprecate the older functions in a future > release. Or not. I think having an 'array_size' and an 'array_length' that behave differently would be legitimately confusing, and I can't think of any alternative function name that would concisely explain the difference in behaviour -- 'array_length_without_the_stupid_nulls' is just too long. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 4 April 2013 01:10, Tom Lane wrote:
> I think though that the upthread argument that we'd have multiple
> interpretations of the same thing is bogus. To me, the core idea that's
> being suggested here is that '{}' should mean a zero-length 1-D array,
> not a zero-D array as formerly. We would still need a way to represent
> zero-D arrays, if only because they'd still exist on-disk in existing
> databases (assuming we're not willing to break pg_upgrade for this).
Tom,
My thought was that on-disk zero-D arrays should be converted into
empty 1-D arrays (with default lower bounds of course) when they are
read by array_recv. Any SQL operation on your zero-D arrays would
therefore resolve as though they were 1-D. A pg_dump/restore would
result in the arrays being 1-D on the restore side. If pg_upgrade
conserves the zero-D array in binary form, that's okay since the
receiving end will just treat it as 1-D out of array_recv anyway.
My intention was that the zero-D array could continue to live
indefinitely in binary form, but would never be observable as such by
any application code.
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 4 April 2013 15:11, Tom Lane wrote: > Brendan Jurd writes: >> My thought was that on-disk zero-D arrays should be converted into >> empty 1-D arrays (with default lower bounds of course) when they are >> read by array_recv. > > Huh? array_recv would not get applied to datums coming off of disk. My mistake, sorry for the noise. > In any case, the whole exercise is pointless if we don't change the > visible behavior of array_dims et al. So I think the idea that this > would be without visible consequence is silly. What's up for argument > is just how much incompatibility is acceptable. I don't know that anyone was suggesting there would be no visible consequences of any kind. I was hoping that we could at least represent on-disk zero-D arrays as though they were 1-D. If that's not going to fly, and we are stuck with continuing to allow zero-D as a valid representation, then perhaps your '[]=' syntax would be the way to proceed. It would not be terribly difficult to rework the patch along those lines, although I have to admit "allow empty arrays with dimensions" is not nearly so satisfying a title as "exorcise zero-dimensional arrays". Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 5 April 2013 07:43, Tom Lane wrote: > Well, if we're going to take that hard a line on it, then we can't > change anything about array data storage or the existing functions' > behavior; which leaves us with either doing nothing at all, or > inventing new functions that have saner behavior while leaving the > old ones in place. And then we are in the awkward position of trying to explain the differences in behaviour between the old and new functions ... presumably with a dash of "for historical reasons" and a sprinkling of "to preserve compatibility" in every other paragraph. The other suggestion that had been tossed around elsewhere upthread was inventing a new type that serves the demand for a straightforward mutable list, which has exactly one dimension, and which may be sensibly empty. Those few who are interested in dimensions >= 2 could keep on using "arrays", with all their backwards-compatible silliness intact, and everybody else could migrate to "lists" at their leisure. I don't hate the latter idea from a user perspective, but from a developer perspective I suspect there are valid objections to be made. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 5 April 2013 13:04, Tom Lane wrote:
> (There's been a remarkable lack of attention to the question
> of spec compliance in this thread, btw. Surely the standard has
> something to say on the matter of zero-length arrays?)
From 4.10 in my draft copy of "Foundation", arrays are one of two
"collection" types (the other being multisets), and:
"A collection is a composite value comprising zero or more elements,
each a value of some data type DT"
"The number of elements in C is the cardinality of C"
"An array is a collection A in which each element is associated with
exactly one ordinal position in A. If n is
the cardinality of A, then the ordinal position p of an element is an
integer in the range 1 (one) ≤ p ≤ n."
The language specifically allows for zero elements, and does not
contemplate multiple dimensions. The specification for the array
constructor syntax (6.36) and array element reference by subscript
(6.23) also make it fairly clear that only 1-D arrays were being
considered.
I'd say we've already gone way off-menu by having multidims. A more
compliant approach would have been to implement arrays as 1-D only,
and then maybe have a separate thing ("matrices"?) for multidims.
While I was in there I noticed CARDINALITY, which would be pretty easy
to add and would at least provide a more productive way to get the
"real" length of an array without disrupting existing functionality:
" ::=
CARDINALITY "
Cheers,
BJ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 5 April 2013 15:05, Tom Lane wrote: > Brendan Jurd writes: >> While I was in there I noticed CARDINALITY, which would be pretty easy >> to add and would at least provide a more productive way to get the >> "real" length of an array without disrupting existing functionality: > > Yeah, that would at least fix the null-result-for-empty-array problem > for that particular functionality. Still, this is ammunition for the > position that null results for empty arrays are just broken. > > BTW ... if you check the archives you will find that we had > cardinality() for a short while, and removed it before 8.4 release, > because we couldn't agree on what it ought to return when given a > multi-dimensional array. I'm afraid that issue is still unresolved. Well for what it's worth I would expect cardinality() to return the total number of elements in the array (per ArrayGetNItems). It's consistent with the spec's identification of an array as a "collection". You can chunk the elements into dimensions however you want, but it's still a collection of elements, and the cardinality is still the number of elements. The "nesting" interpretation doesn't accord with our internal representation, nor with our requirement that multidim arrays be regular, nor with the fact that we can't put an array of texts inside an array of ints. Our array input syntaxes for multidim arrays look nest-ish but what they produce is not nested. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 6 April 2013 01:59, Kevin Grittner wrote: > Brendan Jurd wrote: > >> The language specifically allows for zero elements, and does not >> contemplate multiple dimensions. > > I don't remember anything in the spec which would prohibit the data > type of an array element from itself being an array, however. Indeed it does not prohibit nesting arrays inside other arrays, but the multidim arrays that Postgres allows you to create are not the same thing as nested arrays. I believe that a purely to-spec implementation would allow you to make an array-of-int-arrays, but since each element is its own separate collection there would be no requirement that they have the same cardinality as each other. For example, ARRAY[[1], [2,3], [4,5,6]] is a valid collection per the spec, but Postgres won't let you create this, because Postgres is trying to create a 2-D matrix of integers, rather than a collection of collections of integers. The inability to extend multidim arrays in Postgres is another manifestation of this matrix-oriented design. Extending nested collections is a no-brainer. Extending matrices while ensuring they remain perfectly regular ... not so much. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 7 April 2013 01:43, Kevin Grittner wrote: > Brendan Jurd wrote: >> Indeed it does not prohibit nesting arrays inside other arrays, but >> the multidim arrays that Postgres allows you to create are not the >> same thing as nested arrays. >> > Your interpretation matches mine all around. It is unfortunate > that we have hijacked the standard's syntax for arrays to add a > matrix feature. It really is unfortunate. I wonder if it was done in an attempt to mimic Oracle behaviour. > It seems to leave us without any way forward on > these issues that I like very much. On the specific issue of CARDINALITY, I guess we need to decide whether we are going to pretend that our array/matrix thing is actually nested. I first argued that we should not. But it occurred to me that if we do pretend, it would at least leave the door ajar if we want to do something to make our arrays more nest-like in future, without disrupting the behaviour of CARDINALITY. It is unlikely that we ever would make such a change, but given the intensity of the opposition to any kind of SQL-level behavioural changes we've had on this thread, I don't want to create any more barriers to future efforts to comport with the spec. So how about: * we add CARDINALITY, make it work like array_length(a, 1) except that it returns zero for empty arrays, and * we add array_num_items, which exposes the internal ArrayGetNItems, and returns zero for empty arrays. As in: CARDINALITY(ARRAY[[1,2], [3,4], [5,6]]) => 3 array_num_items(ARRAY[[1,2], [3,4], [5,6]]) => 6 Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: [HACKERS] Should array_length() Return NULL)
On 8 April 2013 16:09, Tom Lane wrote: > Brendan Jurd writes: >> On the specific issue of CARDINALITY, I guess we need to decide >> whether we are going to pretend that our array/matrix thing is >> actually nested. I first argued that we should not. But it occurred >> to me that if we do pretend, it would at least leave the door ajar if >> we want to do something to make our arrays more nest-like in future, >> without disrupting the behaviour of CARDINALITY. > > This seems to be exactly the same uncertainty that we couldn't resolve > back in the 8.4 devel cycle, for exactly the same reasons. I don't see > that the discussion has moved forward any :-( > I had a poke around in the archives, and it seems to me that the major argument that was advanced in favour of making cardinality() return the total number of items was ... we don't have anything that does that yet. That's why I'm proposing we add array_num_items as well -- I do think there should be a function for this, I just don't think cardinality fits the bill. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)
On 9 April 2013 09:24, Josh Berkus wrote: > As much as I have a keen interest in this feature, it isn't (AFAIK) > being considered for 9.3. Given that it's generated a fair amount of > controversy, could we table it until 9.3 beta? There's still plenty of > unresolved 9.3 patches in the queue. No problem. I certainly wasn't expecting it to run for 90 messages when I started out. I'll pipe down for now and resume after the beta. Cheers, BJ -- 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] [GENERAL] to_timestamp() and quarters
On 3 March 2010 14:34, Bruce Momjian wrote:
> Scott Bailey wrote:
>> Tom Lane wrote:
>> > Asher Hoskins writes:
>> >> I can't seem to get to_timestamp() or to_date() to work with quarters,
>> >
>> > The source code says
>> >
>> > * We ignore Q when converting to date because it is not
>> > * normative.
>> > *
>> > * We still parse the source string for an integer, but it
>> > * isn't stored anywhere in 'out'.
>> >
>> > That might be a reasonable position, but it seems like it'd be better to
>> > throw an error than silently do nothing. Anybody know what Oracle does
>> > with this?
>>
>> +1 for throwing error.
>> Oracle 10g throws ORA-01820: format code cannot appear in date input format.
>
> Well, I can easily make it do what you expect, and I don't see many
> error returns in that area of the code, so I just wrote a patch that
> does what you would expect rather than throw an error.
>
> test=> select to_date('2010-1', '-Q');
> to_date
>
> 2010-01-01
> (1 row)
I don't think this is the way to go. Why should the "date" for
quarter 1, 2010 be the first date of that quarter? Why not the last
date? Why not some date in between?
A quarter on its own doesn't assist us in producing a *date* result,
which is after all the purpose of the to_date() function.
I first proposed ignoring the Q field back in 2007 [1]. My motivation
for not throwing an error was that I think the main use-case for
to_date() would be importing data from another system where dates are
in a predictable but non-standard format.
If such a date included the quarter, the user might expect to be able
to include the quarter in his format string.
For example, you're trying to import a date that is written as "Wed
3rd March, Q1 2010". You might give to_date a format string like 'Dy
FMDDTH Month, "Q"Q ' and expect to get the correct answer. If we
start throwing an error on the Q field, then users would have to
resort to some strange circumlocution to get around it.
Having said all of that, it's been pointed out to me in the past that
Oracle compatibility is the main goal of these functions, so if we're
going to change the behaviour of Q in to_date(), I think it should be
in order to move closer to Oracle's treatment. I certainly don't
think we should get back into the business of delivering an exact
answer to an inexact question. So a +1 for throwing the error per Tom
Lane and Scott Bailey.
Cheers,
BJ
[1]
http://archives.postgresql.org/message-id/37ed240d0707170747p4f5c26ffx63fff2b5750c6...@mail.gmail.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] [GENERAL] to_timestamp() and quarters
On 4 March 2010 04:08, Tom Lane wrote:
> Brendan Jurd writes:
>> For example, you're trying to import a date that is written as "Wed
>> 3rd March, Q1 2010". You might give to_date a format string like 'Dy
>> FMDDTH Month, "Q"Q ' and expect to get the correct answer. If we
>> start throwing an error on the Q field, then users would have to
>> resort to some strange circumlocution to get around it.
>
> Hmm. That's an interesting test case: if Q throws an error, there
> doesn't seem to be any way to do it at all, because there is no format
> spec for ignoring non-constant text.
Not entirely true. It's possible, it's just not at all obvious:
=# select to_date('Wed 3rd March, Q1 2010', 'Dy FMDDTH Month, "QQ" ');
to_date
2010-03-03
(1 row)
Anything in a format string which is quoted is ignored. Or to put it
another way, putting stuff in quotes is telling to_date() that the
characters in those positions are not important to you and should not
be used to help construct the date result. It doesn't actually check
that the characters in the source string match what you have put
inside the quotes, it just skips over the quoted number of characters.
I doubt anyone unfamiliar with the source code of the function would
ever devise the above solution, and it's an ugly hack reliant on a
quirk anyway. So a user in-the-field would probably just resort to
running a regexp_replace() over the text in order to strip out the
quarter before passing it to to_date().
> So at the moment my vote is "leave it alone". If we want to throw
> error for Q then we should provide a substitute method of ignoring
> a field. But we could just document Q as ignoring an integer for
> input.
Sounds good to me.
Cheers,
BJ
--
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] FM suffix in to_char Y/YY/YYY still screwy
On 8 April 2010 06:46, Tom Lane wrote: > Not a lot of zero suppression happening there :-(. > > I believe the correct fix is to reduce the year mod 100 (or 10 or 1000) > before feeding it to snprintf, rather than playing games with printing > only part of the result string as the original and current code try to > do. I think you're right. FM should behave as advertised, even for years. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Clobbered parameter names via DECLARE in PL/PgSQL
Hello hackers, It turns out that in a PL/PgSQL function, you can DECLARE a variable using the same name as one of the function parameters. This has the effect of clobbering the parameter, for example: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE foo text; BEGIN RETURN foo; END; $$; SELECT declare_clobber(1); ==> NULL On the other hand, PL/PgSQL does protect against duplicate definitions within DECLARE: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE foo int; foo text; BEGIN RETURN foo; END; $$; ==> ERROR: duplicate declaration at or near "foo" And it also protects against using a DECLAREd name as a parameter alias: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE bar int; bar ALIAS FOR $1; BEGIN RETURN bar; END; $$; ==> ERROR: duplicate declaration at or near "bar" I would suggest that if the user DECLAREs a variable with the same name as a parameter, it is very evidently a programming error, and we should raise the same "duplicate declaration" error. I haven't yet looked at how difficult this would be to fix, but if there are no objections I would like to attempt a patch. Cheers, BJ -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
On 15 April 2012 17:55, Pavel Stehule wrote: > 2012/4/15 Brendan Jurd : >> It turns out that in a PL/PgSQL function, you can DECLARE a variable >> using the same name as one of the function parameters. This has the >> effect of clobbering the parameter, for example: >> ... >> >> I would suggest that if the user DECLAREs a variable with the same >> name as a parameter, it is very evidently a programming error, and we >> should raise the same "duplicate declaration" error. I haven't yet >> looked at how difficult this would be to fix, but if there are no >> objections I would like to attempt a patch. >> > > I disagree - variables and parameters are in different namespace so > you can exactly identify variable and parameter. More - it is > compatibility break. > They may technically be in different namespaces, but the fact that the declared variable quietly goes ahead and masks the parameter locally, seems like a recipe for unexpected consequences. It certainly was in my case, and I doubt I'm the first or the last to make that mistake. Under these conditions, you now have "foo" which refers to the variable, and "declare_clobber.foo" which refers to the parameter. Not exactly a model of clarity, and it's also quite easy to miss the part of the PL/PgSQL docs mentioning this notation. Perhaps it's a failure of imagination on my part, but I can't think of a legitimate reason for a programmer to deliberately use the same name to refer to a declared variable and a function parameter. What would be the benefit? Cheers, BJ -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
On 15 April 2012 18:54, Pavel Stehule wrote: > 2012/4/15 Brendan Jurd : >> Perhaps it's a failure of imagination on my part, but I can't think of >> a legitimate reason for a programmer to deliberately use the same name >> to refer to a declared variable and a function parameter. What would >> be the benefit? > > it depends on level of nesting blocks. For simple functions there > parameter redeclaration is clean bug, but for more nested blocks and > complex procedures, there should be interesting using some local > variables with same identifier like some parameters and blocking > parameter's identifier can be same unfriendly feature like RO > parameters in previous pg versions. > > I understand your motivation well, but solution should be warning, not > blocking. I think. I can accept that ... but I wonder about the implementation of such a warning. Can we raise a WARNING message on CREATE [OR REPLACE] FUNCTION? If so, should there be a way to switch it off? If so, would this be implemented globally, or per-function? Would it be a postgres run-time setting, or an extension to CREATE FUNCTION syntax, or something within the PL/pgSQL code (like Perl's 'use strict')? Cheers, BJ -- 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] Bug tracker tool we need
On 18 April 2012 13:44, Tom Lane wrote: > ... I think you'll find a lot of that data could be mined out of our > historical commit logs already. I know I make a practice of mentioning > "bug #" whenever there is a relevant bug number, and I think other > committers do too. It wouldn't be 100% coverage, but still, if we could > bootstrap the tracker with a few hundred old bugs, we might have > something that was immediately useful, instead of starting from scratch > and hoping it would eventually contain enough data to be useful. Just as a data point, git tells me that there are 387 commits where the commit log message matches '#\d+', and 336 where it matches 'bug #\d+'. Cheers, BJ -- 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] External Open Standards
On 20 May 2012 01:52, Daniel Farina wrote: > The documentation is misleading to the point of our support for ISO > 8601-strict parsing. > > http://archives.postgresql.org/pgsql-hackers/2012-02/msg01237.php > > A very fine point, but I discovered it not out of curiosity, but a > fairly angry user on Twitter. > > We can define the problem away since the space-inclusive format is so > common...so much so, that it is codified in RFC 3339 > (http://www.ietf.org/rfc/rfc3339.txt). The only problem, then, is the > DATESTYLE "ISO" labeling: changing that would be really painful, so > perhaps another solution is to parse the "T" demanded by 8601, > presuming no other details come to light. We may be wandering a bit off-topic from Simon's OP, but I'll bite. We already do *parse* the 'T' in datetime input: postgres=# select timestamp '2012-05-21T15:05'; timestamp - 2012-05-21 15:05:00 (1 row) What we don't do is *output* the 'T', but this is pretty easy to workaround, e.g., to_char(now(), '-MM-DD"T"HH24:MI:SS'). The scope of actually wanting the 'T' is surely pretty minor? I'd be okay with just adding a note in the manual under Date/Time Output to the effect of "Note: ISO 8601 specifies the use of uppercase letter 'T' to separate the date and time. Postgres uses a space for improved readability, in line with other database systems and RFC 3339." Cheers, BJ -- 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] External Open Standards
On 22 May 2012 02:58, Tom Lane wrote: > Peter Eisentraut writes: >> The problem is that people think that "ISO" means ISO 8601, whereas it >> actually means ISO 9075. I can see how that's an easy mistake to make, >> though. > > ... especially since we keep referring to 8601 in our own docs. > Does this mean we should do a global s/8601/9075/ in the docs? Negative on the global replace. Some date/time functions legitimately refer to 8601 (see the page on 'extract' for examples). I had thought that to_char/to_timestamp might refer to 8601 for the "ISO week date" feature, but there doesn't appear to be an explicit mention on that page. Perhaps there should be? The text in 8.5.2. Date/Time Output seems to be the culprit here. It (thrice) names 8601 as the governing standard for the ISO output datestyle, and it has some ancillary wording that would become awkward if we just subbed in 9075 instead. Currently the first few sentences read: "The output format of the date/time types can be set to one of the four styles ISO 8601, SQL (Ingres), traditional POSTGRES (Unix date format), or German. The default is the ISO format. (The SQL standard requires the use of the ISO 8601 format. The name of the "SQL" output format is a historical accident.) " I would suggest something like: "The output format of the date/time types can be set to one of the four styles ISO, SQL (Ingres), traditional POSTGRES (Unix date format), or German. The default is the ISO format, which refers to ISO 9075, the SQL standard. (The name of the "SQL" output format is a historical accident, it does not refer to the SQL standard.) " The note I suggested earlier would probably need to change in light of the above, too. Perhaps along these lines: "Note: the output format specified by ISO 9075 is identical to that specified in ISO 8601, except that the uppercase letter 'T' separating the date part from the time part is replaced with a space for the sake of readability." Cheers, BJ -- 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] External Open Standards
On 24 May 2012 05:30, Peter Eisentraut wrote: > On mån, 2012-05-21 at 15:34 +1000, Brendan Jurd wrote: >> I'd be okay with just adding a note in the manual under Date/Time >> Output to the effect of "Note: ISO 8601 specifies the use of uppercase >> letter 'T' to separate the date and time. Postgres uses a space for >> improved readability, in line with other database systems and RFC >> 3339." > > But that is wrong. We use the space because SQL says so. The reason we > have all those other formats is for readability or consistency or some > secondary standard. But the format of the ISO format is exactly what > the SQL standard says, without any other considerations. > Did you miss my follow-on message on this thread? You are responding to a message that has been superceded. I saw what you wrote earlier, and updated my suggestions for the docs 2 days ago. Please see http://archives.postgresql.org/message-id/cadxjzo34ma54imgd7ruadcc-r6lq7w-ta8gqwvahkua7bsv...@mail.gmail.com Cheers, BJ -- 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] Easy way to verify gitignore files?
On 23 September 2010 11:28, Abhijit Menon-Sen wrote: >> This seems pretty dangerous, especially for people who are willing to >> rely on "git commit -a" :-( > > There is no danger. "git commit -a" will commit changes to files that > match .gitignore but are already in the repository. (I vaguely remember > that there were bugs in this regard in old versions of git, but it's not > a problem with any recent version AFAIK.) > Right; .gitignore patterns are only applied to untracked files. Once a file is tracked by git, you can try to gitignore it all you like, it won't have any effect. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Docs for archive_cleanup_command are poor
Hi folks, I have just set up HS+SR for the first time, and for the most part, the docs were excellent. The one exception for me was the discussion of archive_cleanup_command. This is a pretty important part of constructing a healthy standby server, and IMO the docs don't give it the treatment it deserves. Under "25.2.4. Setting Up a Standby Server", we have: "You can use archive_cleanup_command to prune the archive of files no longer needed by the standby." ... then a few paragraphs later ... "If you're using a WAL archive, its size can be minimized using the archive_cleanup_command option to remove files that are no longer required by the standby server. Note however, that if you're using the archive for backup purposes, you need to retain files needed to recover from at least the latest base backup, even if they're no longer needed by the standby." So there are a couple of brief mentions of what archive_cleanup_command is for, but nothing about how it works, no exampes of how to use it, and no links at all. Contrast how we deal with archive_command, restore_command and primary_conninfo. I'd like to suggest a few ways we could improve on this: 1. Remove the former paragraph. It's stranded out there on its own in the middle of some unrelated text, and doesn't say anything of substance not also said in the latter paragraph. 2. Include an example archive_cleanup_command in the recovery.conf example snippet. 3. Link to 26.1 which actually explains how a_c_c works. 4. Mention, and link to, pg_archivecleanup from both 25.2.4 and 26.1. This is the utility that most newcomers to WAL archiving will want to use, so it's rather weird of us not to advertise it. I'm willing to write a patch for this, but I thought I'd raise the suggestions on-list first, before getting too invested. So, please comment if you have an opinion on this. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Keywords in pg_hba.conf should be field-specific
Hi folks, One of the speedbumps I hit when setting up HS+SR was naming the user the slave would connect as for streaming replication. At first I picked 'replication', which seemed quite natural to me (and I don't doubt will seem natural to others as well). When I started up the slave, I got this error: FATAL: could not connect to the primary server: FATAL: no pg_hba.conf entry for replication connection from host "192.168.21.10", user "replication", SSL on Bzzzt. Wrong. There *was* such an entry in pg_hba.conf. I wasted a lot of time checking my conf files for typos before I wondered whether there might be something wrong with using 'replication' as a username. I changed the username to 'streamrep' and it all started working perfectly. I understand that 'replication' is a keyword as far as the database name is concerned, but I was surprised to find that it was treated as a keyword in the username field also. I had a look in src/backend/libpq/hba.c, and next_token() appears to be completely naive about this. 'replication' (along with 'all', 'sameuser', 'samegroup' and 'samerole') is treated as a keyword wherever it appears, not just in the field where it is relevant. next_token() appends a newline to the end of the 'replication' username token, and that's why the entry doesn't match my connection attempt. I suspect this is going to trip a lot of people up. We could just document it and tell people that if they want to use 'replication' as a username, they'd better quote it in pg_hba.conf. But I'd prefer to actually solve the problem. We could do this by teaching next_token to be sensitive to which field it is parsing, and limit keyword detection appropriately. Or, if that's awkward, we could teach parse_hba_line to ignore keyword markers where the keyword is not applicable. The more I think about it, the more I incline towards the latter. Thoughts? Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [DOCS] [HACKERS] Docs for archive_cleanup_command are poor
On 14 October 2010 08:45, Robert Haas wrote: > Is someone working on a patch? Yes, I will prepare a patch to get us started. Cheers, BJ -- 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] Docs for archive_cleanup_command are poor
On 12 October 2010 23:28, Fujii Masao wrote: > On Sat, Oct 9, 2010 at 10:04 AM, Brendan Jurd wrote: >> I have just set up HS+SR for the first time, and for the most part, >> the docs were excellent. The one exception for me was the discussion >> of archive_cleanup_command. This is a pretty important part of >> constructing a healthy standby server, and IMO the docs don't give it >> the treatment it deserves. ... > Agreed. > > And, ISTM that we should mention that we must not just specify > pg_archivecleanup in archive_cleanup_command when there are multiple > standby servers. This is because, in that case, we must calculate > the oldest restart point in those standbys and delete the archived > WAL files according to that point. As promised, here is a patch to try to address $SUBJECT. Summary of changes: In 25.2.4. "Setting Up a Standby Server": * Get rid of the extraneous short paragraph, * move the full-size paragraph up to where the now-extinct short para was, * add an archive_cleanup_command to the example recovery.conf, * flesh out the wording, * add links to 26.1 and F.22. In 26.1. "Archive recovery settings": * Add detail to the description of how it works, * add an example recovery.conf snippet, * per Fujii-san's comment, indicate that multi-standby setups require more finesse, * link to F.22. In F.22. "pg_archivecleanup": * Edit and clarify wording, * standardise label for the argument, * again indicate the multi-standby issue, * link to 25.2. I'll drop this onto the next open commitfest. If it passes muster, it sure wouldn't hurt to backpatch it to 9.0. Cheers, BJ *** a/doc/src/sgml/high-availability.sgml --- b/doc/src/sgml/high-availability.sgml *** *** 681,691 protocol to make nodes agree on a serializable transactional order. - You can use archive_cleanup_command to prune the archive of - files no longer needed by the standby. - - - If you're setting up the standby server for high availability purposes, set up WAL archiving, connections and authentication like the primary server, because the standby server will work as a primary server after --- 681,686 *** *** 697,708 protocol to make nodes agree on a serializable transactional order. --- 692,716 + If you're using a WAL archive, its size can be minimized using the parameter to remove files that are no + longer required by the standby server. + The pg_archivecleanup utility is designed specifically to + be used with archive_cleanup_command in typical single-standby + configurations, see . + Note however, that if you're using the archive for backup purposes, you + need to retain files needed to recover from at least the latest base + backup, even if they're no longer needed by the standby. + + + A simple example of a recovery.conf is: standby_mode = 'on' primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass' restore_command = 'cp /path/to/archive/%f %p' trigger_file = '/path/to/trigger_file' + archive_cleanup_command = 'pg_archivecleanup /path/to/archive %r' *** *** 712,725 trigger_file = '/path/to/trigger_file' the primary to allow them to be connected simultaneously. - - If you're using a WAL archive, its size can be minimized using - the archive_cleanup_command option to remove files that are - no longer required by the standby server. Note however, that if you're - using the archive for backup purposes, you need to retain files needed - to recover from at least the latest base backup, even if they're no - longer needed by the standby. - --- 720,725 *** a/doc/src/sgml/pgarchivecleanup.sgml --- b/doc/src/sgml/pgarchivecleanup.sgml *** *** 8,17 ! pg_archivecleanup is designed to cleanup an archive when used ! as an archive_cleanup_command when running with ! standby_mode = on. pg_archivecleanup can ! also be used as a standalone program to clean WAL file archives. --- 8,18 ! pg_archivecleanup is designed to be used as an ! archive_cleanup_command to clean up WAL file archives when ! running as a standby server (see ). ! pg_archivecleanup can also be used as a standalone program to ! clean WAL file archives. *** *** 39,58 server to use pg_archivecleanup, put this into its recovery.conf configuration file: ! archive_cleanup_command = 'pg_archivecleanup archiveDir %r' !where archiveDir is the directory from which WAL segment !files should be restored. !When used within archive_cleanup_command,
Re: [HACKERS] Docs for archive_cleanup_command are poor
On 15 October 2010 05:33, Simon Riggs wrote: > On Fri, 2010-10-15 at 02:24 +1100, Brendan Jurd wrote: >> I'll drop this onto the next open commitfest. If it passes muster, it >> sure wouldn't hurt to backpatch it to 9.0. > > Committed. Not sure there's anything there worth backpatching? There > aren't any doc bugs there. > Thanks for the commit Simon. Agreed that there are no doc bugs. The reason I suggested a backpatch is that I'm concerned that a lot of people are going to be approaching the whole Standby topic for the first time with 9.0, so it would be nice to give those folks an accessible account of how archive_cleanup_command is meant to be used. I was also working from the assumption that the "we only packpatch bug fixes" policy applied to the code, not so much to the documentation. If I was in error about that, well fair enough then. Cheers, BJ -- 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] Keywords in pg_hba.conf should be field-specific
On 13 October 2010 00:28, Fujii Masao wrote: > On Sat, Oct 9, 2010 at 11:13 AM, Brendan Jurd wrote: >> I understand that 'replication' is a keyword as far as the database >> name is concerned, but I was surprised to find that it was treated as >> a keyword in the username field also. I had a look in >> src/backend/libpq/hba.c, and next_token() appears to be completely >> naive about this. 'replication' (along with 'all', 'sameuser', >> 'samegroup' and 'samerole') is treated as a keyword wherever it >> appears, not just in the field where it is relevant. next_token() >> appends a newline to the end of the 'replication' username token, and >> that's why the entry doesn't match my connection attempt. >> >> I suspect this is going to trip a lot of people up. We could just >> document it and tell people that if they want to use 'replication' as >> a username, they'd better quote it in pg_hba.conf. But I'd prefer to >> actually solve the problem. > > Agreed. We should address that. > Hi folks, Per the above discussion, I've prepared a patch to make keywords in pg_hba.conf field-specific. This patch takes a least-resistance approach to solving the problem. next_token() continues to blithely append a newline character to any token which *might* be a keyword, but the patch teaches hba_parse_line() to be a bit more savvy about whether to take next_token's word for it. In the Database field, hba_parse_line() acknowledges 'all', 'sameuser', 'samegroup', 'samerole' and 'replication' as legitimate keywords. Anything else is considered not a keyword, and if there is a trailing newline it is stripped from the token (by just reassigning it to NUL). Likewise, in the Role field, only 'all' is considered legitimate. In the Host field, we allow 'samehost' and 'samenet'. The bottom line is that this makes 'replication' safe to use as a username, no quoting required. It seemed a little bit inelegant to allow next_token() to mark up a keyword, only to ignore that marking later on, but as the number of fields in each line is not decided until we parse it, there was no sensible way to teach next_token() which field it was evaluating. Added to the November CF. Cheers, BJ -- 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] Keywords in pg_hba.conf should be field-specific
On 17 October 2010 01:52, Brendan Jurd wrote:
> Per the above discussion, I've prepared a patch to make keywords in
> pg_hba.conf field-specific.
>
Try New and Improved This Message (tm), now with attachment!
Cheers,
BJ
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***
*** 855,861 parse_hba_line(List *line, int line_num, HbaLine *parsedline)
line_num,
HbaFileName)));
return false;
}
! parsedline->database = pstrdup(lfirst(line_item));
/* Get the role. */
line_item = lnext(line_item);
--- 855,873
line_num,
HbaFileName)));
return false;
}
! token = pstrdup(lfirst(line_item));
!
! /* Filter out inapplicable keywords for the database field. */
! if (token[strlen(token) - 1] == '\n'
! && strcmp(token, "all\n") != 0
! && strcmp(token, "sameuser\n") != 0
! && strcmp(token, "samerole\n") != 0
! && strcmp(token, "samegroup\n") != 0
! && strcmp(token, "replication\n") != 0)
! {
! token[strlen(token) - 1] = '\0';
! }
! parsedline->database = token;
/* Get the role. */
line_item = lnext(line_item);
***
*** 868,874 parse_hba_line(List *line, int line_num, HbaLine *parsedline)
line_num,
HbaFileName)));
return false;
}
! parsedline->role = pstrdup(lfirst(line_item));
if (parsedline->conntype != ctLocal)
{
--- 880,894
line_num,
HbaFileName)));
return false;
}
! token = pstrdup(lfirst(line_item));
!
! /* Filter out inapplicable keywords for the role field. */
! if (token[strlen(token) - 1] == '\n'
! && strcmp(token, "all\n") != 0)
! {
! token[strlen(token) - 1] = '\0';
! }
! parsedline->role = token;
if (parsedline->conntype != ctLocal)
{
***
*** 904,909 parse_hba_line(List *line, int line_num, HbaLine *parsedline)
--- 924,937
/* need a modifiable copy of token */
token = pstrdup(token);
+ /*
+* Filter out any remaining keywords, as the only valid
keywords
+* for this context ('samehost' and 'samenet') have
already been
+* handled above.
+*/
+ if (token[strlen(token) - 1] = '\n')
+ token[strlen(token) - 1] = '\0';
+
/* Check if it has a CIDR suffix and if so isolate it */
cidr_slash = strchr(token, '/');
if (cidr_slash)
--
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] all keyword for pg_hba.conf host column
On 16 October 2010 21:56, Peter Eisentraut wrote: > As a small addition to the pg_hba.conf host name feature, I figured it > would be useful to allow "all" in the host column, instead of having to > write 0.0.0.0/0 and ::/0. Patch attached. > Cool. And, for what it's worth, this doesn't conflict at all with the field-specific keywords patch I just submitted. Cheers, BJ -- 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] Keywords in pg_hba.conf should be field-specific
On 17 October 2010 02:27, Alvaro Herrera wrote: > Hmm. Would it be possible to list keywords _applicable_ to each field, > and have these passed down to next_token by the caller instead? This > seems backwards, but I'm not sure if the other way is really workable. > Short answer: I don't think it is workable, or I would have done it that way in the first place. Full answer: The problem is that pg_hba.conf doesn't have a fixed structure. Each line can be 4, 5 or 6 fields (not including the final 'options' field) long, and which of these structures apply to any given line isn't decided until parse_hba_line goes to work on it. At the time that next_token gets called, we have no way of knowing which field is currently being tokenised, at least not without doing some serious rearrangement of hba.c, so that it tokenises and then parses one token at a time, instead of tokenising the whole file and then parsing all the lines. Cheers, BJ -- 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] Keywords in pg_hba.conf should be field-specific
On 17 October 2010 09:59, Tom Lane wrote: > Brendan Jurd writes: >> At the time that next_token gets called, we have no way of knowing >> which field is currently being tokenised, at least not without doing >> some serious rearrangement of hba.c, so that it tokenises and then >> parses one token at a time, instead of tokenising the whole file and >> then parsing all the lines. > > Good point. Maybe the correct fix is to remember whether each token was > quoted or not, so that keyword detection can be done safely after the > initial lexing. I still think that the current method is impossibly > ugly ... > I'm happy to revise the patch on that basis. Any suggestions about how to communicate the 'quotedness' of each token? We could make each token a struct consisting of the token itself, plus a boolean flag to indicate whether it had been quoted. Does that work for you? Cheers, BJ -- 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] Simplifying replication
On 19 October 2010 11:16, Josh Berkus wrote: > 4. I can start a new replica off the master by running a single command-line > utility on the standby and giving it connection information to the master. > Using this connection, it should be able to start a backup snapshot, copy > the entire database and any required logs, and then come up in standby mode. > All that should be required for this is one or two highport connections to > the master. No recovery.conf file is required, or exists. Having just configured replication for the first time with 9.0, I agree in general with your whole message, but in particular, I want to give a double-thumbs-up to the above. Resolving this one item would subtract a great deal of pain -- and potential for error -- from the process. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Integer input functions for date and timestamp
Hi folks, In my own databases, I've been using a couple of C functions that might be useful to the wider community. They are very simple date/timestamp constructors that take integers as their arguments. Nothing fancy, but very convenient and *much* faster than using a SQL or PL/pgSQL workaround. The offering is analogous to mktime() in C/PHP, the standard datetime constructors in Python, and Perl's Time::Local. The function signatures pretty much speak for themselves: date(year int, month int, day int) returns date datetime(year int, month int, day int, hour int, minute int, second int) returns timestamp Without these functions (or some variation), a user wishing to construct a date from integers can only assemble the date into a string and then put that string through postgres' datetime parser, which is totally perverse. Is there any interest in adding this to core, or failing that, contrib? If so I'd be happy to provide a patch including the functions themselves and some attendant documentation. I'm not wedded to the function names or argument order, and I realise a fully realised offering would need to include a variant for 'timestamp with time zone'. Cheers, BJ -- 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] Integer input functions for date and timestamp
On 23 October 2010 05:58, Peter Geoghegan wrote: > On 22 October 2010 19:45, Brendan Jurd wrote: >> Without these functions (or some variation), a user wishing to >> construct a date from integers can only assemble the date into a >> string and then put that string through postgres' datetime parser, >> which is totally perverse. > > What's wrong with to_timestamp() and to_date()? Sure, your functions > might be marginally faster, but I don't think that it's likely to be a > very performance sensitive area. > Hi Peter, The answer to your question is in the paragraph I quoted from my OP above. to_timestamp() and to_date() don't offer any non-retarded way to get from integer values to datetime values. They are great if you are coming from text, but if you already have integers they are lame. Perhaps an example would be constructive. Would you rather do this: datetime(2010, 10, 23, 6, 11, 0) or this: to_date(2010::text || '-' || 10::text || '-' || 23::text || ' ' || 6::text || ':' || 11::text || ':' || 0::text, '-MM-DD HH24:MI:SS') The performance increase is nice, but as you say, this isn't likely to be in a performance critical path. The main benefits are convenience, simplicity and readability. Cheers, BJ -- 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] Integer input functions for date and timestamp
On 23 October 2010 06:15, Josh Berkus wrote: > a) you'd need to rename these. I'm open to that. What names would you propose? > b) we'd also want the inverse of these, which would be extremely useful. Not a problem. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Segfault in 9.0 inlining SRF
Hi folks,
I have encountered a reproducible segfault in Postgres, and confirmed
it in 9.0.1 and HEAD on three separate machines. The bug was not
present in 8.4. I've attached a copy of the SQL script I have been
using to induce the segfault.
With asserts enabled, I get a failed assertion:
TRAP: FailedAssertion("!(((list) == ((List *) ((void *)0)) ||
Node*)((list)))->type) == T_List)))", File: "list.c", Line: 130)
If I attach gdb to the server process, here's my backtrace as the
assert is fired:
#0 0x7f0247f7f1b5 in raise () from /lib/libc.so.6
#1 0x7f0247f805e0 in abort () from /lib/libc.so.6
#2 0x006fbf6d in ExceptionalCondition (conditionName=,
errorType=, fileName=,
lineNumber=) at assert.c:57
#3 0x005acdbb in lappend (list=0xc49e60, datum=0xc562f0) at list.c:130
#4 0x005e536f in record_plan_function_dependency
(glob=0xb6bb78, funcid=16385)
at setrefs.c:1738
#5 0x005f378f in inline_set_returning_function
(root=0xb6c140, rte=0xb6bc10)
at clauses.c:4318
#6 0x005eaced in inline_set_returning_functions (root=0xb6c140)
at prepjointree.c:438
#7 0x005e48b8 in subquery_planner (glob=, parse=0xb6b9f0,
parent_root=0x0, hasRecursion=0 '\000', tuple_fraction=,
subroot=) at planner.c:341
#8 0x005e5051 in standard_planner (parse=0xb6b9f0, cursorOptions=0,
boundParams=0x0) at planner.c:200
#9 0x006414b6 in pg_plan_query (querytree=0xb6b9f0, cursorOptions=0,
boundParams=0x0) at postgres.c:757
#10 0x006415b4 in pg_plan_queries (querytrees=,
cursorOptions=0, boundParams=0x0) at postgres.c:816
#11 0x006425d0 in exec_simple_query (query_string=)
at postgres.c:981
#12 0x00643498 in PostgresMain (argc=,
argv=, username=) at
postgres.c:3869
#13 0x00607ba1 in BackendRun () at postmaster.c:3556
#14 BackendStartup () at postmaster.c:3241
#15 ServerLoop () at postmaster.c:1432
#16 0x0060a4ad in PostmasterMain (argc=3, argv=0xb645e0) at
postmaster.c:1093
#17 0x005a8b70 in main (argc=3, argv=0xb645d0) at main.c:188
And then the postmaster outputs:
LOG: server process (PID 18343) was terminated by signal 6: Aborted
LOG: terminating any other active server processes
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back
the current transaction and exit, because another server process
exited abnormally and possibly corrupted shared memory.
HINT: In a moment you should be able to reconnect to the database and
repeat your command.
LOG: all server processes terminated; reinitializing
LOG: database system was interrupted; last known up at 2010-10-25 07:02:45 EST
LOG: database system was not properly shut down; automatic recovery in progress
LOG: consistent recovery state reached at 0/62C768
LOG: redo starts at 0/62C768
LOG: invalid magic number in log file 0, segment 0, offset 6520832
LOG: redo done at 0/6370B8
LOG: database system is ready to accept connections
I had a go at investigating the cause of the bug, but didn't have much
success as I'm not at all familiar with the guts of the optimizer.
Cheers,
BJ
segfault.sql
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] [BUGS] Segfault in 9.0 inlining SRF
On 25 October 2010 07:36, Tom Lane wrote:
> Looks like the invalItems list has been clobbered:
>
> (gdb) p *root->glob->invalItems
> $6 = {type = 2139062143, length = 2139062143, head = 0x7f7f7f7f,
> tail = 0x7f7f7f7f}
>
> I'm guessing it was modified in the temporary memory context and not
> properly copied out to the parent context when we finished inlining
> the function.
I note that the error goes away if I do any of the following with my test case:
a) define the SRF as having no arguments, or
b) make the argument passed into the SRF a constant rather than a
call to a function, or
c) make the argument function (year) itself take no arguments, or
d) make the argument passed to year a constant rather than current_date, or
e) define year as VOLATILE, or
f) put the call to segfault_setof into a subquery in the FROM clause.
So this only manifests under a pretty specific type of query tree; a
function call as the argument to another function call, which is an
argument to an inlinable SRF in FROM.
Cheers,
BJ
--
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] [BUGS] Segfault in 9.0 inlining SRF
On 25 October 2010 07:36, Tom Lane wrote:
> Brendan Jurd writes:
>> I have encountered a reproducible segfault in Postgres ...
>
> Looks like the invalItems list has been clobbered:
>
> (gdb) p *root->glob->invalItems
> $6 = {type = 2139062143, length = 2139062143, head = 0x7f7f7f7f,
> tail = 0x7f7f7f7f}
>
> I'm guessing it was modified in the temporary memory context and not
> properly copied out to the parent context when we finished inlining
> the function.
>
Hi Tom,
Thanks for the hint; I found that the attached patch resolved my
specific segfault, but I am wondering whether it goes far enough. The
patch just copies invalItems up out of the temporary context before it
is deleted. Could there also be changes to other elements of
PlannerGlobal that need to be saved? Should we in fact be copying out
the whole of PlannerGlobal each time, and would that necessitate a new
copyfunc for it?
Cheers,
BJ
diff --git a/src/backend/optimizer/util/clauses.c
b/src/backend/optimizer/util/clauses.c
index 13e89ec..b39ebb6 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4300,12 +4300,13 @@ inline_set_returning_function(PlannerInfo *root,
RangeTblEntry *rte)
fexpr->args);
/*
-* Copy the modified query out of the temporary memory context, and
clean
-* up.
+* Copy the modified query, and the possibly-altered global invalidation
+* list, out of the temporary memory context, and clean up.
*/
MemoryContextSwitchTo(oldcxt);
querytree = copyObject(querytree);
+ root->glob->invalItems = copyObject(root->glob->invalItems);
MemoryContextDelete(mycxt);
error_context_stack = sqlerrcontext.previous;
--
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] [BUGS] Segfault in 9.0 inlining SRF
On 26 October 2010 03:42, Tom Lane wrote: > Brendan Jurd writes: >> Thanks for the hint; I found that the attached patch resolved my >> specific segfault, but I am wondering whether it goes far enough. > > Well, it definitely doesn't go far enough, because the invalItems list > has to be restored to its original state if we fail to inline at some > point after calling eval_const_expressions. Point taken. > I thought about whether we need something more general, but for the > moment I think this is sufficient; eval_const_expressions has only > very limited reason to examine the PlannerInfo data at all, and less > reason to modify it. Copying the whole structure would be overkill. > Moreover, it wouldn't do anything to improve modularity anyhow: this > function would still have to know which parts of the structure to copy > back to the top level, and which not. So it'd still need to know > quite a bit about exactly what eval_const_expressions is doing. That makes sense to me. This whole business of passing around global pointers while switching memory contexts seems like an optimal breeding-ground for bugs. It would be nice to come up with a more ... well, "global" way to manage PlannerGlobal. To me it suggests something along the lines of a dedicated MemoryContext in which PlannerGlobal and all its members live, and you operate on PlannerGlobal by calling methods, rather than by directly twiddling its pointers. But, that is probably way over the top for this, given its narrow area of effect. Cheers, BJ -- 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] Keywords in pg_hba.conf should be field-specific
On 18 October 2010 01:19, Tom Lane wrote: > Brendan Jurd writes: >> On 17 October 2010 09:59, Tom Lane wrote: >>> Good point. Maybe the correct fix is to remember whether each token was >>> quoted or not, so that keyword detection can be done safely after the >>> initial lexing. I still think that the current method is impossibly >>> ugly ... > >> I'm happy to revise the patch on that basis. Any suggestions about >> how to communicate the 'quotedness' of each token? We could make each >> token a struct consisting of the token itself, plus a boolean flag to >> indicate whether it had been quoted. Does that work for you? > > Seems reasonable. I had the idea of a parallel list of booleans in the > back of my mind, but a list of structs is probably easier to understand, > and to extend further if necessary. > Okay, I've taken the red pill and I'm finding out how deep the rabbit hole goes ... The logical structure of pg_hba.conf is a set of lines, each line containing a set of fields, each field containing a set of tokens. The way the existing implementation handles this is to create a list of lines containing sublists of fields, containing comma-separated strings for the set of tokens, with newlines embedded next to tokens which might be keywords. The tokeniser breaks apart the comma-separated tokens ... and then reassembles them into a comma-separated string. Which the db/role matching functions then have to break apart *again*. In order to keep track of whether each individual token was quoted, I first need to impose some sanity here. Rather than using a magical string for each field, I intend to use a List of HbaToken structs which explicitly note whether quoting was used. Introducing an extra List level does mean a bit more work copying and freeing, and it makes the patch really quite intrusive. I have to touch a lot of lines in hba.c, but I think the additional clarity is worth it. If nobody dissuades me from this approach I hope to post a patch in a couple of days. Cheers, BJ -- 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] How can we tell how far behind the standby is?
On 6 November 2010 05:46, Josh Berkus wrote: > I'm continuing in my efforts now to document how to deploy and manage > replication on our wiki. One of the things a DBA needs to do is to use > pg_current_xlog_location() (and related functions) to check how far > behind the master the standby is. > > However, there's some serious problems with that: > > (1) comparing these numbers is quite mathematically complex -- and, for > that matter, undocumented. > Our solution to this was to strip the slash out of the numbers and then feed them to `bc` for comparison. The shell script for our zabbix item looks something like this: #!/bin/bash errval=-1 primary=$(psql -At -h $1 -p $2 -c "SELECT replace(pg_current_xlog_location(), '/', '');" postgres) standby=$(psql -At -h $3 -p $4 -c "SELECT replace(pg_last_xlog_receive_location(), '/', '');" postgres) if [ -n "$primary" -a -n "$standby" ] then echo $(echo "ibase=16; obase=10; $primary-$standby" | bc) else echo $errval fi I'm posting this snippet a) in the hopes that it might help others, and b) by way of agreement with Josh's point. Requiring every user who wants to monitor replication to set something like this up for themselves is ... not awesome. Cheers, BJ -- 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] Getting a bug tracker for the Postgres project
On 29 May 2011 14:04, Tom Lane wrote: > Anything that even pretends to be a bug tracker will do that. The > real question is, who is going to keep it up to date? GSM has the > right point of view here: we need at least a couple of people who > are willing to invest substantial amounts of time, or it's not going > to go anywhere. Seeing that we can barely manage to keep the mailing > list moderator positions staffed, I'm not hopeful. > Well the good news is that first-pass triage of bug reports can be done by pretty much anybody who is a moderately experienced postgres user; they don't even need to be a hacker. They just need to know when to send back a RTFM link, when to say "you didn't tell us your PG version" / "post your query" / "post your explain analyse" / "post your show all", and when to kick the bug report up to a sage hacker. It's not glamorous work, but it is a very accessible way to contribute, without the need to block out hours at a time. A bug wrangler could very readily log in, sort out reports for 20 minutes and then go do something else with the rest of their day. Cheers, BJ -- 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] Getting a bug tracker for the Postgres project
On 31 May 2011 11:52, Robert Haas wrote: > I have used RT and I found that the > web interface was both difficult to use and unwieldly for tickets > containing large numbers of messages. A big loud "ditto" from me on this point. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] DOCS: SGML identifier may not exceed 44 characters
Hi folks, I was working on a little docs patch today, and when I tried to `make`, openjade choked on an identifier in information_schema.sgml, which is very much unrelated to my changes: openjade:information_schema.sgml:828:60:Q: length of name token must not exceed NAMELEN (44) Here is a trivial patch to shut openjade up. This particular id does not appear to be referred to anywhere else in the docs yet. The identifier appears to have been introduced in commit 2e2d56fea97f43cf8c40a87143bc10356e4ed4d4 on Feb 9 this year. I'm using openjade 1.3.2. Cheers, BJ diff --git a/doc/src/sgml/information_schema.sgml b/doc/src/sgml/information_schema.sgml index 2febb4c..5fdbd51 100644 --- a/doc/src/sgml/information_schema.sgml +++ b/doc/src/sgml/information_schema.sgml @@ -825,7 +825,7 @@ - + collation_character_set_applicability -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [DOCS] [HACKERS] DOCS: SGML identifier may not exceed 44 characters
On 1 June 2011 06:36, Peter Eisentraut wrote: > It looks like the original DocBook distribution has a limit of 44, but > someone patched it to 256 on your installation. > > But it seems like no one else has seen this problem yet, so it's quite > suspicious, since surely people have built the documentation in the last > few months. > The relevant value on my machine seems to be: /usr/share/sgml/docbook/sgml-dtd-4.2/docbook.dcl:81: NAMELEN44 This file belongs to the Gentoo package app-text/docbook-sgml-dtd 4.2-r2, which is the current stable version for the 4.2 slot. I would hazard to suggest that nobody else is seeing this problem because I'm the only one building the docs on Gentoo =) Still, the 44 character limit does seem to be per the upstream distribution, and the identifier I patched above is the first one to violate it. Are there any solid reasons we shouldn't just comply with it? Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fwd: Keywords in pg_hba.conf should be field-specific
On 16 June 2011 00:22, Pavel Stehule wrote: > I try to apply your patch, but it is finished with some failed hinks. > > Please, can you refresh your patch Hi Pavel, Thanks for taking a look. I have attached v2 of the patch, as against current HEAD. I've also added the new patch to the CF app. I look forward to your comments. Cheers, BJ hba-keywords-v2.diff.bz2 Description: BZip2 compressed 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] minor patch submission: CREATE CAST ... AS EXPLICIT
On 22 May 2011 07:27, Fabien COELHO wrote: > > Hello Tom, > >>> Add "AS EXPLICIT" to "CREATE CAST" This gives a name to the default case >>> of "CREATE CAST", which creates a cast which must be explicitely invoked. >> >> I'm not sure this is a good idea. The CREATE CAST syntax is in the SQL >> standard, and this isn't it. Now I realize that we've extended that >> statement already to cover some functionality that's not in the >> standard, but that doesn't mean we should create unnecessarily >> nonstandard syntax for cases that are in the standard. > > The standard provides only one case, so "CAST" is good enough a name. > > Once you start creating alternatives with distinct semantics, then it helps > to give the initial one a name as well to be able to discuss them with > something else that "the remaining case", or "when there is no option", > especially as there is something to discuss. > > Note that the standard is still supported just the same, and the > documentation already underlines that "AS *" stuff is a pg extension, > nothing is really changed. Maybe the documentation could be clearer about > where the standard stops and where extensions start, even now without an "AS > EXPLICIT" clause. > >> If a commercial vendor did that, wouldn't you castigate them for trying to >> create vendor lock-in? > > I'm more concerned with explaining things to students, and its good to have > words and logic for that. > > With respect to the standard, it seems good enough to me if (1) the standard > is well supported and (2) the documentation clearly says which parts are > extensions. If you really want to keep to the standard, then do not offer > any extension. > > Moreover, this stuff is really minor compared to RULEs or many other things > specifics to pg, and the "lock-in" is light, you just have to remove "AS > EXPLICIT" to get away, no big deal. > Hi Fabien, I'm taking a look at this patch for the commitfest. On first reading of the patch, it looked pretty sensible to me, but I had some trouble applying it to HEAD: error: patch failed: doc/src/sgml/ref/create_cast.sgml:20 error: doc/src/sgml/ref/create_cast.sgml: patch does not apply error: patch failed: src/backend/parser/gram.y:499 error: src/backend/parser/gram.y: patch does not apply error: patch failed: src/include/parser/kwlist.h:148 error: src/include/parser/kwlist.h: patch does not apply error: patch failed: src/test/regress/expected/create_cast.out:27 error: src/test/regress/expected/create_cast.out: patch does not apply error: patch failed: src/test/regress/sql/create_cast.sql:27 error: src/test/regress/sql/create_cast.sql: patch does not apply Perhaps the patch could use a refresh? Also, for what it's worth, I buy into the argument for adding AS EXPLICIT. This stuff is all an extension to the SQL standard already; it might as well have a well-rounded syntax. Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 18 June 2011 13:43, Alvaro Herrera wrote: > Is this really a WIP patch? I'm playing a bit with it currently, seems > fairly sane. > In this case, the WIP designation is meant to convey "warning: only casual testing has beeen done". I tried it out with various permutations of pg_hba.conf, and it worked as advertised in those tests, but I have not made any attempt to formulate a more rigorous testing regimen. In particular I haven't tested that the more exotic authentication methods still work properly, and I can't recall whether I tested recursive file inclusion and group membership. Is that a wrongful use of the WIP designation? Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 06:06, Alvaro Herrera wrote: > Excerpts from Alvaro Herrera's message of lun jun 20 12:19:37 -0400 2011: >> Excerpts from Pavel Stehule's message of lun jun 20 11:34:25 -0400 2011: >> >> > b) probably you can simplify a memory management using own two >> > persistent memory context - and you can swap it. Then functions like >> > free_hba_record, clean_hba_list, free_lines should be removed. >> >> Yeah, I reworked the patch with something like that over the weekend. >> Not all of it though. I'll send an updated version shortly. > > Here it is, please let me know what you think. I took the liberty of > cleaning up some things that were clearly historical leftovers. > Okay, yeah, the MemoryContext approach is far more elegant than what I had. Boy was I ever barking up the wrong tree. Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 11:11, Alvaro Herrera wrote: > I realize I took out most of the fun of this patch from you, but -- are > you still planning to do some more exhaustive testing of it? I checked > some funny scenarios (including include files and groups) but it's not > all that unlikely that I missed some cases. I also didn't check any > auth method other than ident, md5 and trust, 'cause I don't have what's > required for anything else. (It's a pity that the regression tests > don't exercise anything else.) I've pretty much tested all the things I can think to test, and I'm in the same boat as you re auth methods. If there are bugs, I think they will be of the obscure kind, that only reveal themselves under peculiar circumstances. Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 13:51, Pavel Stehule wrote: > I have one question. I can't find any rules for work with tokens, etc, > where is quotes allowed and disallowed? > > I don't see any other issues. I'm not sure I understand your question, but quotes are allowed anywhere and they always act to remove any special meaning the token might otherwise have had. It's much like quoting a column name in SQL. I didn't change any of the hba parsing rules, so escaping and whatnot should all work the way it did before. The only difference should be that after parsing, keywords will only be evaluated for fields where they matter. Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 21 June 2011 14:34, Pavel Stehule wrote: > I don't understand to using a macro > > #define token_is_keyword(t, k) (!t->quoted && strcmp(t->string, k) == 0) > > because you disallowed a quoting? Well, a token can only be treated as a special keyword if it is unquoted. As an example, in the 'database' field, the bare token 'replication' is a keyword meaning the pseudo-database for streaming rep. Whereas the quoted token "replication" would mean a real database which is called 'replication'. Likewise, the bare token 'all' in the username field is a keyword -- it matches any username. Whereas the quoted token "all" would only match a user named 'all'. Therefore, token_is_keyword only returns true where the token matches the given string as is also unquoted. Does that make sense? Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 22 June 2011 00:47, Tom Lane wrote: > Alvaro Herrera writes: >> Excerpts from Pavel Stehule's message of mar jun 21 00:59:44 -0400 2011: >>> because >>pamservice<< - is known keyword, but 'pamservice' is some >>> literal without any mean. You should to use a makro token_is_keyword >>> more often. > >> Yeah, I wondered about this too (same with auth types, i.e. do we accept >> quoted "hostssl" and so on or should that by rejected?). I opted for >> leaving it alone, but maybe this needs to be fixed. (Now that I think >> about it, what we should do first is verify whether it works with quotes >> in the unpatched code). > > AFAICS, this is only important in places where the syntax allows either > a keyword or an identifier. If only a keyword is possible, there is no > value in rejecting it because it's quoted. And, when you do the test, > I think you'll find that it would be breaking hba files that used to > work (though admittedly, it's doubtful that there are any such in the > field). Yes, I agree, and this was my thinking when I came up against this while writing the original patch. It doesn't help to treat "hostssl" differently than hostssl, because quoted identifiers are meaningless in that context. Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 22 June 2011 14:01, Pavel Stehule wrote: > ook, now is clean, so this is majority opinion. > > Please, can you send a final patch. I don't have any further changes to add to Alvaro's version 3, which is already up on the CF app. Cheers, BJ -- 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] Fwd: Keywords in pg_hba.conf should be field-specific
On 24 June 2011 03:48, Alvaro Herrera wrote: > I have touched next_token() and next_token_expand() a bit more, because > it seemed to me that they could be simplified further (the bit about > returning the comma in the token, instead of being a boolean return, > seemed strange). Please let me know what you think. Cool. I didn't like the way it returned the comma either, but I thought it would be best if I kept the changes in my patch to a minimum. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
