Re: [HACKERS] improved DefElem list processing

2016-09-10 Thread Peter Eisentraut
On 8/22/16 10:28 AM, Alvaro Herrera wrote: > Peter Eisentraut wrote: > >> I'm not happy that utils/acl.h has prototypes for aclchk.c, because >> acl.h is included all over the place. Perhaps I should make a >> src/include/catalog/aclchk.c to clean that up. > > I've been bothered by that too in t

Re: [HACKERS] improved DefElem list processing

2016-09-06 Thread Peter Eisentraut
On 9/6/16 7:23 PM, Tom Lane wrote: > I'm curious where you are on that? I find myself needing to whack around > this processing in CREATE EXTENSION, but I don't want to create a merge > problem for you if you're close to committing. I have committed what I have for now. Thanks. -- Peter Eisent

Re: [HACKERS] improved DefElem list processing

2016-09-06 Thread Tom Lane
Peter Eisentraut writes: > Here are two WIP patches to improve the DefElem list processing that is > used by many utility commands. > One factors out the duplicate checks, which are currently taking up a > lot of space with duplicate code. I haven't applied this everywhere > yet, but the patch s

Re: [HACKERS] improved DefElem list processing

2016-08-22 Thread Alvaro Herrera
Peter Eisentraut wrote: > I'm not happy that utils/acl.h has prototypes for aclchk.c, because > acl.h is included all over the place. Perhaps I should make a > src/include/catalog/aclchk.c to clean that up. I've been bothered by that too in the past. +1 for the cleanup. -- Álvaro Herrera

Re: [HACKERS] improved DefElem list processing

2016-08-22 Thread Michael Paquier
On Mon, Aug 22, 2016 at 10:41 PM, Pavel Stehule wrote: > 1. This patch introduce location in DefElement node, and inject ParserState > to SQL commands, where ParserState was not used. It allows to show the > position of an error. This patch is not small, but almost changes are > trivial. > > 2. Th

Re: [HACKERS] improved DefElem list processing

2016-08-22 Thread Pavel Stehule
Hi 2016-08-11 17:32 GMT+02:00 Peter Eisentraut < peter.eisentr...@2ndquadrant.com>: > On 8/5/16 11:25 AM, Peter Eisentraut wrote: > > On 8/4/16 2:21 PM, Tom Lane wrote: > >> Forgot to mention: seems like you should have added a location > >> argument to makeDefElem. > > > > I was hesitating to d

Re: [HACKERS] improved DefElem list processing

2016-08-11 Thread Peter Eisentraut
On 8/5/16 11:25 AM, Peter Eisentraut wrote: > On 8/4/16 2:21 PM, Tom Lane wrote: >> Forgot to mention: seems like you should have added a location >> argument to makeDefElem. > > I was hesitating to do that lest it break extensions or something, but I > guess we break bigger things than that all t

Re: [HACKERS] improved DefElem list processing

2016-08-05 Thread Peter Eisentraut
On 8/4/16 2:21 PM, Tom Lane wrote: > Forgot to mention: seems like you should have added a location > argument to makeDefElem. I was hesitating to do that lest it break extensions or something, but I guess we break bigger things than that all the time. I'll change it. -- Peter Eisentraut

Re: [HACKERS] improved DefElem list processing

2016-08-05 Thread Peter Eisentraut
On 8/4/16 2:08 PM, Tom Lane wrote: > +1 on the general idea, but I wonder if it's a problem that you replaced > an O(N) check with an O(N^2) check. I don't think there are any commands > that would be likely to have so many options that this would become a > serious issue, but ... I'll run some t

Re: [HACKERS] improved DefElem list processing

2016-08-04 Thread Tom Lane
I wrote: > Peter Eisentraut writes: >> The other adds a location field to the DefElem node. > +1 for sure, lots of places where that would be a good thing > (the duplicate check itself, for starters). Forgot to mention: seems like you should have added a location argument to makeDefElem.

Re: [HACKERS] improved DefElem list processing

2016-08-04 Thread Tom Lane
Peter Eisentraut writes: > Here are two WIP patches to improve the DefElem list processing that is > used by many utility commands. > One factors out the duplicate checks, which are currently taking up a > lot of space with duplicate code. I haven't applied this everywhere > yet, but the patch s