Thread: Re: [HACKERS] Performance testing of COPY (SELECT) TO
Hi, what's the problem with COPY view TO, other than you don't like it? :-) That was the beginning and is used in production according to the original authors. > I also broke the check for a FOR UPDATE clause. Not sure where but it > must be easy to fix :-) I'd do it myself but I'm heading to bed right > now. Fixed. > I also wanted to check these hunks in your patch, which I didn't like > very much: > > -ERROR: column "a" of relation "test" does not exist > +ERROR: column "a" does not exist It was because of too much code sharing. I fixed it by passing the relation name to CopyGetAttnums() in the relation case, so the other regression tests aren't bothered now. The docs and the regression test is modified according to your version. Best regards, Zoltán Böszörményi
Attachment
Böszörményi Zoltán wrote: > Hi, > > what's the problem with COPY view TO, other than you don't like it? :-) The problem is that it required a ugly piece of code. Not supporting it means we can keep the code nice. The previous discussion led to this conclusion anyway so I don't know why we are debating it again. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera wrote: > Böszörményi Zoltán wrote: > >> Hi, >> >> what's the problem with COPY view TO, other than you don't like it? :-) >> > > The problem is that it required a ugly piece of code. Not supporting it > means we can keep the code nice. The previous discussion led to this > conclusion anyway so I don't know why we are debating it again. > > What is so ugly about it? I haven't looked at the code, but I am curious to know. I also don't recall the consensus being quite so clear cut. I guess there is a case for saying that if it's not allowed then you know that "COPY relname TO" is going to be fast. But, code aesthetics aside, the reasons for disallowing it seem a bit thin, to me. cheers andrew
Andrew Dunstan írta: > Alvaro Herrera wrote: >> Böszörményi Zoltán wrote: >> >>> Hi, >>> >>> what's the problem with COPY view TO, other than you don't like it? :-) >>> >> >> The problem is that it required a ugly piece of code. Not supporting it >> means we can keep the code nice. The previous discussion led to this >> conclusion anyway so I don't know why we are debating it again. >> >> > > What is so ugly about it? I haven't looked at the code, but I am > curious to know. > > I also don't recall the consensus being quite so clear cut. I guess > there is a case for saying that if it's not allowed then you know that > "COPY relname TO" is going to be fast. But, code aesthetics aside, the > reasons for disallowing it seem a bit thin, to me. > > cheers > > andrew > I would say the timing difference between "COPY table TO" and "COPY (SELECT * FROM table) TO" was noise, so it's not even faster. And an updatable VIEW *may* allow COPY view FROM... Best regards, Zoltán Böszörményi
Zoltan Boszormenyi wrote: > Andrew Dunstan írta: > >Alvaro Herrera wrote: > >>Böszörményi Zoltán wrote: > >> > >>>what's the problem with COPY view TO, other than you don't like it? :-) > >> > >>The problem is that it required a ugly piece of code. Not supporting it > >>means we can keep the code nice. The previous discussion led to this > >>conclusion anyway so I don't know why we are debating it again. > > > >What is so ugly about it? I haven't looked at the code, but I am > >curious to know. It used a "SELECT * FROM %s" string that was passed back to the parser. > >I also don't recall the consensus being quite so clear cut. I guess > >there is a case for saying that if it's not allowed then you know that > >"COPY relname TO" is going to be fast. But, code aesthetics aside, the > >reasons for disallowing it seem a bit thin, to me. > > I would say the timing difference between > "COPY table TO" and "COPY (SELECT * FROM table) TO" > was noise, so it's not even faster. Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). > And an updatable VIEW *may* allow COPY view FROM... May I remind you that we've been in feature freeze for four weeks already? Now it's *not* the time to be drooling over cool features that would be nice to have. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
> Remember that we were talking about supporting views, not tables. And
> if a view uses a slow query then you are in immediate danger of having a
> slow COPY. This may not be a problem but it needs to be discussed and
> an agreement must be reached before introducing such a change (and not
> during feature freeze).
>
>
>
this will definitely be the case - however, this is not what it was made
for. it has not been made to be fast but it has been made to fulfill
some other task. the reason why this has been implemented is: consider a
large scale database containing hundreds of gigs of data. in our special
case we have to export in a flexible way. the data which has to be
exported comes from multiple tables (between 3 and 7 depending on the
data we are looking at in this project. the export has to be performed
in a flexible way and it needs certain parameters. defining tmp tables
and store the data in there is simply not "nice" at all. in most cases
exports want to transform data on the fly - speed is not as important as
flexibility here.
so in my view the speed argument does not matter. if somebody passes a
stupid query to copy he will get stupid runtimes - just like on ordinary
sql. however, we can use COPY's capabilities to format / escape data to
make exports more flexible. so basically it is a win.
best regards,
hans
--
Cybertec Geschwinde & Schönig GmbH
Schöngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at
Hans-Juergen Schoenig wrote: > > >Remember that we were talking about supporting views, not tables. And > >if a view uses a slow query then you are in immediate danger of having a > >slow COPY. This may not be a problem but it needs to be discussed and > >an agreement must be reached before introducing such a change (and not > >during feature freeze). > > this will definitely be the case - however, this is not what it was made > for. it has not been made to be fast but it has been made to fulfill > some other task. the reason why this has been implemented is: consider a > large scale database containing hundreds of gigs of data. in our special > case we have to export in a flexible way. the data which has to be > exported comes from multiple tables (between 3 and 7 depending on the > data we are looking at in this project. the export has to be performed > in a flexible way and it needs certain parameters. defining tmp tables > and store the data in there is simply not "nice" at all. in most cases > exports want to transform data on the fly - speed is not as important as > flexibility here. My question is, if we allow this: copy (select * from view) to stdout; (or to a file, whatever), is it enough for you? Or would you insist on also having copy view to stdout; ? We can, and the posted patch does, support the first form, but not the second. In fact I deliberately removed support for the second form for Zoltán's patch because it uglifies the surrounding code. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera írta: > Zoltan Boszormenyi wrote: > >> Andrew Dunstan írta: >> >>> Alvaro Herrera wrote: >>> >>>> Böszörményi Zoltán wrote: >>>> >>>> >>>>> what's the problem with COPY view TO, other than you don't like it? :-) >>>>> >>>> The problem is that it required a ugly piece of code. Not supporting it >>>> means we can keep the code nice. The previous discussion led to this >>>> conclusion anyway so I don't know why we are debating it again. >>>> >>> What is so ugly about it? I haven't looked at the code, but I am >>> curious to know. >>> > > It used a "SELECT * FROM %s" string that was passed back to the parser. > > >>> I also don't recall the consensus being quite so clear cut. I guess >>> there is a case for saying that if it's not allowed then you know that >>> "COPY relname TO" is going to be fast. But, code aesthetics aside, the >>> reasons for disallowing it seem a bit thin, to me. >>> >> I would say the timing difference between >> "COPY table TO" and "COPY (SELECT * FROM table) TO" >> was noise, so it's not even faster. >> > > Remember that we were talking about supporting views, not tables. And > if a view uses a slow query then you are in immediate danger of having a > slow COPY. This may not be a problem but it needs to be discussed and > an agreement must be reached before introducing such a change (and not > during feature freeze). > COPY relname TO meant tables _and_ views to me. My previous tsting showed no difference between COPY table TO and COPY (SELECT * FROM table) TO. Similarly a slow query defined in the view should show no difference between COPY view TO and COPY (SELECT * FROM view) TO. And remember, Bruce put the original COPY view TO patch into the unapplied queue, without the SELECT feature. Rewriting COPY view TO internally to COPY (SELECT * FROM view) TO is very straightforward, even if you think it's ugly. BTW, why is it ugly if I call raw_parser() from under src/backend/parser/*.c ? It is on a query distinct to the query the parser is currently running. Or is it the recursion that bothers you? It's not a possible infinite recursion. >> And an updatable VIEW *may* allow COPY view FROM... >> > > May I remind you that we've been in feature freeze for four weeks > already? Now it's *not* the time to be drooling over cool features that > would be nice to have Noted. However, as the COPY view TO is a straight internal rewrite, a COPY view FROM could also be. Even if it's a long term development. I wasn't proposing delaying beta. Best regards, Zoltán Böszörményi
Alvaro Herrera <alvherre@commandprompt.com> writes:
> My question is, if we allow this:
> copy (select * from view) to stdout;
> (or to a file, whatever), is it enough for you? Or would you insist on
> also having
> copy view to stdout;
> ?
> We can, and the posted patch does, support the first form, but not the
> second. In fact I deliberately removed support for the second form for
> Zolt�n's patch because it uglifies the surrounding code.
Personally, I have no moral objection to supporting the second form
as a special case of the general COPY-from-select feature, but if it
can't be done without uglifying the code then I'd agree with dropping
it. I guess the question is whether the uglification is intrinsic or
just a result of being descended from a poor original implementation.
The feature-freeze argument seems not relevant, given that the code
we had on the feature-freeze date did both things.
Has this patch settled to the point where I can review it, or is it
still in motion?
regards, tom lane
Alvaro Herrera wrote:
> Hans-Juergen Schoenig wrote:
>
>>> Remember that we were talking about supporting views, not tables. And
>>> if a view uses a slow query then you are in immediate danger of having a
>>> slow COPY. This may not be a problem but it needs to be discussed and
>>> an agreement must be reached before introducing such a change (and not
>>> during feature freeze).
>>>
>> this will definitely be the case - however, this is not what it was made
>> for. it has not been made to be fast but it has been made to fulfill
>> some other task. the reason why this has been implemented is: consider a
>> large scale database containing hundreds of gigs of data. in our special
>> case we have to export in a flexible way. the data which has to be
>> exported comes from multiple tables (between 3 and 7 depending on the
>> data we are looking at in this project. the export has to be performed
>> in a flexible way and it needs certain parameters. defining tmp tables
>> and store the data in there is simply not "nice" at all. in most cases
>> exports want to transform data on the fly - speed is not as important as
>> flexibility here.
>>
>
> My question is, if we allow this:
>
> copy (select * from view) to stdout;
>
> (or to a file, whatever), is it enough for you? Or would you insist on
> also having
>
> copy view to stdout;
> ?
>
>
i would say that "copy view to stdout" is just some syntactic sugar (to
me at least). the important thing is that we add the flexibility of
SELECT to it. a view is nothing else than a rule on SELECT anyway. to be
honest i never thought about views when creating this copy idea.
however, i think it is not bad to have it because i have seen a couple
of times already that tables turn into views when new features are added
to an existing data structure . if we support copy on views this means
that exports can stay as they are even if the data structure is changed
in that way.
however, if people think that views are not needed that way it is still
a good solution as views are not the basic reason why this new
functionality is a good thing to have.
many thanks,
hans
--
Cybertec Geschwinde & Schönig GmbH
Schöngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at
Zoltan Boszormenyi wrote: > Alvaro Herrera írta: > >Remember that we were talking about supporting views, not tables. And > >if a view uses a slow query then you are in immediate danger of having a > >slow COPY. This may not be a problem but it needs to be discussed and > >an agreement must be reached before introducing such a change (and not > >during feature freeze). > > COPY relname TO meant tables _and_ views to me. > My previous tsting showed no difference between > COPY table TO and COPY (SELECT * FROM table) TO. > Similarly a slow query defined in the view should show > no difference between COPY view TO and > COPY (SELECT * FROM view) TO. The difference is that we are giving a very clear distinction between a table and a view. If we don't support the view in the direct COPY, but instead insist that it be passed via a SELECT query, then the user will be aware that it may be slow. "relname" at this point may mean anything -- are you supporting sequences and toast tables as well? > And remember, Bruce put the original COPY view TO > patch into the unapplied queue, without the SELECT > feature. All sort of junk enters that queue so that's not an argument. (Not meant to insult Bruce -- I'm just saying that he doesn't filter stuff. We've had patches rejected from the queue before plenty of times.) > Rewriting COPY view TO internally to > COPY (SELECT * FROM view) TO is very > straightforward, even if you think it's ugly. > BTW, why is it ugly if I call raw_parser() > from under src/backend/parser/*.c ? > It is on a query distinct to the query the parser > is currently running. Or is it the recursion > that bothers you? It's not a possible infinite > recursion. It's ugly because you are forcing the system to parse something that was already parsed. On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > My question is, if we allow this: > > copy (select * from view) to stdout; > > (or to a file, whatever), is it enough for you? Or would you insist on > > also having > > copy view to stdout; > > ? > > > We can, and the posted patch does, support the first form, but not the > > second. In fact I deliberately removed support for the second form for > > Zoltán's patch because it uglifies the surrounding code. > > Personally, I have no moral objection to supporting the second form > as a special case of the general COPY-from-select feature, but if it > can't be done without uglifying the code then I'd agree with dropping > it. I guess the question is whether the uglification is intrinsic or > just a result of being descended from a poor original implementation. I'm quite sure you could refactor things as needed to support the "COPY view" case reasonably. It's just beyond what I'd do during the current freeze. It seems I'm alone on the "view may be slow" camp. If I lost that argument I have no problem accepting that. > The feature-freeze argument seems not relevant, given that the code > we had on the feature-freeze date did both things. Actually IIRC the patch on the queue only did the "COPY view" stuff, not the COPY select. (Thanks go to Zoltan for properly morphing the patch). > Has this patch settled to the point where I can review it, or is it > still in motion? Personally I'm finished doing the cleanup I wanted to do. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
>>> Remember that we were talking about supporting views, not tables. And
>>> if a view uses a slow query then you are in immediate danger of having a
>>> slow COPY. This may not be a problem but it needs to be discussed and
>>> an agreement must be reached before introducing such a change (and not
>>> during feature freeze).
>>>
>> COPY relname TO meant tables _and_ views to me.
>> My previous tsting showed no difference between
>> COPY table TO and COPY (SELECT * FROM table) TO.
>> Similarly a slow query defined in the view should show
>> no difference between COPY view TO and
>> COPY (SELECT * FROM view) TO.
>>
>
> The difference is that we are giving a very clear distinction between a
> table and a view. If we don't support the view in the direct COPY, but
> instead insist that it be passed via a SELECT query, then the user will
> be aware that it may be slow.
>
what kind of clever customers do you have in the US? ;) i would never
say something like that here :).
i see your point and i think it is not a too bad idea. at least some
folks might see that there is no voodoo going on ...
> "relname" at this point may mean anything -- are you supporting
> sequences and toast tables as well?
>
>
good point ...
>
>
> It's ugly because you are forcing the system to parse something that
> was already parsed.
>
definitely an argument for dropping the view stuff ...
> On the other hand I don't see why you are arguing in favor of a useless
> feature whose coding is dubious; you can have _the same thing_ with nice
> code and no discussion.
>
>
what are you referring to?
hans
--
Cybertec Geschwinde & Schönig GmbH
Schöngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at
Alvaro Herrera írta: > Zoltan Boszormenyi wrote: > >> Alvaro Herrera írta: >> > > >>> Remember that we were talking about supporting views, not tables. And >>> if a view uses a slow query then you are in immediate danger of having a >>> slow COPY. This may not be a problem but it needs to be discussed and >>> an agreement must be reached before introducing such a change (and not >>> during feature freeze). >>> >> COPY relname TO meant tables _and_ views to me. >> My previous tsting showed no difference between >> COPY table TO and COPY (SELECT * FROM table) TO. >> Similarly a slow query defined in the view should show >> no difference between COPY view TO and >> COPY (SELECT * FROM view) TO. >> > > The difference is that we are giving a very clear distinction between a > table and a view. If we don't support the view in the direct COPY, but > instead insist that it be passed via a SELECT query, then the user will > be aware that it may be slow. > It still can be documented with supporting the COPY view TO syntax. But COPY view (col1, col2, ...) TO may still be useful even if the COPY (SELECT ...) (col1, col2, ...) TO is pointless. [1] > "relname" at this point may mean anything -- are you supporting > sequences and toast tables as well? > Well, not really. :-) >> And remember, Bruce put the original COPY view TO >> patch into the unapplied queue, without the SELECT >> feature. >> > > All sort of junk enters that queue so that's not an argument. (Not > meant to insult Bruce -- I'm just saying that he doesn't filter stuff. > We've had patches rejected from the queue before plenty of times.) > OK. :-) >> Rewriting COPY view TO internally to >> COPY (SELECT * FROM view) TO is very >> straightforward, even if you think it's ugly. >> BTW, why is it ugly if I call raw_parser() >> from under src/backend/parser/*.c ? >> It is on a query distinct to the query the parser >> is currently running. Or is it the recursion >> that bothers you? It's not a possible infinite >> recursion. >> > > It's ugly because you are forcing the system to parse something that > was already parsed. > Well, to be true to the word, during parsing COPY view TO the parser never saw SELECT * FROM view. > On the other hand I don't see why you are arguing in favor of a useless > feature whose coding is dubious; you can have _the same thing_ with nice > code and no discussion. > Because of [1] and because Mr. Schoenig's arguments about changing schemas. Best regards, Zoltán Böszörményi
Hans-Juergen Schoenig wrote: > >It's ugly because you are forcing the system to parse something that > >was already parsed. > > definitely an argument for dropping the view stuff ... On the other hand, it's quite possible that this could be made to work _without_ doing black magic (which would be OK by me). > >On the other hand I don't see why you are arguing in favor of a useless > >feature whose coding is dubious; you can have _the same thing_ with nice > >code and no discussion. > > what are you referring to? The fact that the direct "copy view" feature is just syntactic sugar over "copy (select * from view)". The latter we can have without discussion -- from me, that is :-) -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Zoltan Boszormenyi wrote: > Alvaro Herrera írta: > But COPY view (col1, col2, ...) TO may still be > useful even if the COPY (SELECT ...) (col1, col2, ...) TO > is pointless. [1] Hum, I don't understand what you're saying here -- are you saying that you can't do something with the first form, that you cannot do with the second? > >It's ugly because you are forcing the system to parse something that > >was already parsed. > > Well, to be true to the word, during parsing COPY view TO > the parser never saw SELECT * FROM view. Hmm! The COPY view stuff stopped working when I changed back the "relation" case. Your patch changed it so that instead of flowing as RangeVar all the way to the copy.c code, the parser changed it into a "select * from %s" query, and then stashed the resulting Query node into the "query" %case. (So what was happening was that the Relation case was never %used). I reverted this. > >On the other hand I don't see why you are arguing in favor of a useless > >feature whose coding is dubious; you can have _the same thing_ with nice > >code and no discussion. > > Because of [1] and because Mr. Schoenig's arguments > about changing schemas. Yeah, that argument makes sense to me as well. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
>> On the other hand I don't see why you are arguing in favor of a useless
>> feature whose coding is dubious; you can have _the same thing_ with nice
>> code and no discussion.
>>
>
> Because of [1] and because Mr. Schoenig's arguments
> about changing schemas.
>
first of all; hans is enough - skip the mr ;)
i think changing schema is a good argument but we could sacrifice that
for the sake of clarity and clean code. i am not against keeping it but
i can understand the argument against views. i always preferred select.
mr hans ;)
--
Cybertec Geschwinde & Schönig GmbH
Schöngrabern 134; A-2020 Hollabrunn
Tel: +43/1/205 10 35 / 340
www.postgresql.at, www.cybertec.at
Alvaro Herrera írta:
> Zoltan Boszormenyi wrote:
>
>> Alvaro Herrera írta:
>>
>
>
>> But COPY view (col1, col2, ...) TO may still be
>> useful even if the COPY (SELECT ...) (col1, col2, ...) TO
>> is pointless. [1]
>>
>
> Hum, I don't understand what you're saying here -- are you saying that
> you can't do something with the first form, that you cannot do with the
> second?
>
Say you have a large often used query.
Would you like to retype it every time
or just create a view? Later you may want to
export only a subset of the fields...
My v8 had the syntax support for
COPY (SELECT ...) (col1, col2, ...) TO
and it was actually working. In your v9
you rewrote the syntax parsing so that
feature was lost in translation.
>>> It's ugly because you are forcing the system to parse something that
>>> was already parsed.
>>>
>> Well, to be true to the word, during parsing COPY view TO
>> the parser never saw SELECT * FROM view.
>>
>
> Hmm!
>
> The COPY view stuff stopped working when I changed back the "relation"
> case. Your patch changed it so that instead of flowing as RangeVar all
> the way to the copy.c code, the parser changed it into a "select * from
> %s" query, and then stashed the resulting Query node into the "query"
> %case. (So what was happening was that the Relation case was never
> %used). I reverted this.
>
Well, the VIEW case wasn't supported before
so I took the opportunity to transform it in
analyze.c which you deleted as being ugly.
>>> On the other hand I don't see why you are arguing in favor of a useless
>>> feature whose coding is dubious; you can have _the same thing_ with nice
>>> code and no discussion.
>>>
>> Because of [1] and because Mr. Schoenig's arguments
>> about changing schemas.
>>
>
> Yeah, that argument makes sense to me as well.
>
So, may I put it back? :-)
Also, can you suggest anything cleaner than
calling raw_parser("SELECT * FROM view")?
Zoltan Boszormenyi wrote:
> Alvaro Herrera írta:
> >Zoltan Boszormenyi wrote:
> >
> >>Alvaro Herrera írta:
> >
> >>But COPY view (col1, col2, ...) TO may still be
> >>useful even if the COPY (SELECT ...) (col1, col2, ...) TO
> >>is pointless. [1]
> >>
> >
> >Hum, I don't understand what you're saying here -- are you saying that
> >you can't do something with the first form, that you cannot do with the
> >second?
>
> Say you have a large often used query.
> Would you like to retype it every time
> or just create a view? Later you may want to
> export only a subset of the fields...
>
> My v8 had the syntax support for
>
> COPY (SELECT ...) (col1, col2, ...) TO
> and it was actually working. In your v9
> you rewrote the syntax parsing so that
> feature was lost in translation.
Interesting. I didn't realize this was possible -- obviously I didn't
test it (did you have a test for it in the regression tests? I may have
missed it). In fact, I deliberately removed the column list from the
grammar, because it can certainly be controlled inside the SELECT, so I
thought there was no reason the support controlling it in the COPY
column list.
I don't think it's difficult to put it back. But this has nothing to do
with COPY view, does it?
> >>>On the other hand I don't see why you are arguing in favor of a useless
> >>>feature whose coding is dubious; you can have _the same thing_ with nice
> >>>code and no discussion.
> >>>
> >>Because of [1] and because Mr. Schoenig's arguments
> >>about changing schemas.
> >
> >Yeah, that argument makes sense to me as well.
>
> So, may I put it back? :-)
> Also, can you suggest anything cleaner than
> calling raw_parser("SELECT * FROM view")?
I think at this point is someone else's judgement whether you can put it
back or not. Tom already said that he doesn't object to the feature per
se; no one else seems opposed to the feature per se, in fact.
Now, I don't really see _how_ to do it in nice code, so no, I don't have
any suggestion for you. You may want to give the pumpkin to Tom so that
he gives the patch the finishing touches (hopefully making it support
the "COPY view" feature as well).
If it were up to me, I'd just commit it as is (feature-wise -- more
thorough review is still needed) and revisit the COPY view stuff in 8.3
if there is demand.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera írta:
> Zoltan Boszormenyi wrote:
>
>> Alvaro Herrera írta:
>>
>>> Zoltan Boszormenyi wrote:
>>>
>>>
>>>> Alvaro Herrera írta:
>>>>
>>>
>>>
>>>> But COPY view (col1, col2, ...) TO may still be
>>>> useful even if the COPY (SELECT ...) (col1, col2, ...) TO
>>>> is pointless. [1]
>>>>
>>>>
>>> Hum, I don't understand what you're saying here -- are you saying that
>>> you can't do something with the first form, that you cannot do with the
>>> second?
>>>
>> Say you have a large often used query.
>> Would you like to retype it every time
>> or just create a view? Later you may want to
>> export only a subset of the fields...
>>
>> My v8 had the syntax support for
>>
>> COPY (SELECT ...) (col1, col2, ...) TO
>> and it was actually working. In your v9
>> you rewrote the syntax parsing so that
>> feature was lost in translation.
>>
>
> Interesting. I didn't realize this was possible -- obviously I didn't
> test it (did you have a test for it in the regression tests? I may have
> missed it). In fact, I deliberately removed the column list from the
> grammar, because it can certainly be controlled inside the SELECT, so I
> thought there was no reason the support controlling it in the COPY
> column list.
>
Yes, it was even documented. I thought about having
queries stored statically somewhere (not in views) and
being able to use only part of the result.
> I don't think it's difficult to put it back. But this has nothing to do
> with COPY view, does it?
>
No, but it may be confusing seeing
COPY (SELECT ) (col1, col2, ...) TO
instead of COPY (SELECT col1, col2, ...) TO.
With the COPY VIEW (col1, col2, ...) TO syntax
it may be cleaner from the user's point of view.
Together with the changing schemas argument
it gets more and more tempting.
>>>>> On the other hand I don't see why you are arguing in favor of a useless
>>>>> feature whose coding is dubious; you can have _the same thing_ with nice
>>>>> code and no discussion.
>>>>>
>>>>>
>>>> Because of [1] and because Mr. Schoenig's arguments
>>>> about changing schemas.
>>>>
>>> Yeah, that argument makes sense to me as well.
>>>
>> So, may I put it back? :-)
>> Also, can you suggest anything cleaner than
>> calling raw_parser("SELECT * FROM view")?
>>
>
> I think at this point is someone else's judgement whether you can put it
> back or not. Tom already said that he doesn't object to the feature per
> se; no one else seems opposed to the feature per se, in fact.
>
> Now, I don't really see _how_ to do it in nice code, so no, I don't have
> any suggestion for you. You may want to give the pumpkin to Tom so that
> he gives the patch the finishing touches (hopefully making it support
> the "COPY view" feature as well).
>
> If it were up to me, I'd just commit it as is (feature-wise -- more
> thorough review is still needed) and revisit the COPY view stuff in 8.3
> if there is demand.
>
OK, I will put it back as it was in v8
keeping all your other cleanup and
let Bruce and Tom decide.
(BTW, is there anyone as high-ranking as them,
or the "committee" is a duumvirate? :-) )
Zoltan Boszormenyi wrote: > >I think at this point is someone else's judgement whether you can put it > >back or not. Tom already said that he doesn't object to the feature per > >se; no one else seems opposed to the feature per se, in fact. > > > >Now, I don't really see _how_ to do it in nice code, so no, I don't have > >any suggestion for you. You may want to give the pumpkin to Tom so that > >he gives the patch the finishing touches (hopefully making it support > >the "COPY view" feature as well). > > > >If it were up to me, I'd just commit it as is (feature-wise -- more > >thorough review is still needed) and revisit the COPY view stuff in 8.3 > >if there is demand. > > OK, I will put it back as it was in v8 > keeping all your other cleanup and > let Bruce and Tom decide. Hum, are you going to put back the original cruft to support copy view? I suggest you don't do that. > (BTW, is there anyone as high-ranking as them, > or the "committee" is a duumvirate? :-) ) There is a "core", there are committers, there are "major developers", and there are "contributors". This is documented in the developer's page on the website, though the committers group is not documented anywhere. (Most, but not all, of Core are also committers. Some Major Developers are committers as well). There is no committee. The closer you get to that, is people vocal enough on pgsql-hackers. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera írta: > Zoltan Boszormenyi wrote: > > >>> I think at this point is someone else's judgement whether you can put it >>> back or not. Tom already said that he doesn't object to the feature per >>> se; no one else seems opposed to the feature per se, in fact. >>> >>> Now, I don't really see _how_ to do it in nice code, so no, I don't have >>> any suggestion for you. You may want to give the pumpkin to Tom so that >>> he gives the patch the finishing touches (hopefully making it support >>> the "COPY view" feature as well). >>> >>> If it were up to me, I'd just commit it as is (feature-wise -- more >>> thorough review is still needed) and revisit the COPY view stuff in 8.3 >>> if there is demand. >>> >> OK, I will put it back as it was in v8 >> keeping all your other cleanup and >> let Bruce and Tom decide. >> > > Hum, are you going to put back the original cruft to support copy view? > I suggest you don't do that. > Well, the other way around is to teach heap_open() to use views. Brrr. Would it be any cleaner?
Zoltan Boszormenyi wrote: > Alvaro Herrera írta: > >Zoltan Boszormenyi wrote: > > > > > >>>I think at this point is someone else's judgement whether you can put it > >>>back or not. Tom already said that he doesn't object to the feature per > >>>se; no one else seems opposed to the feature per se, in fact. > >>> > >>>Now, I don't really see _how_ to do it in nice code, so no, I don't have > >>>any suggestion for you. You may want to give the pumpkin to Tom so that > >>>he gives the patch the finishing touches (hopefully making it support > >>>the "COPY view" feature as well). > >>> > >>>If it were up to me, I'd just commit it as is (feature-wise -- more > >>>thorough review is still needed) and revisit the COPY view stuff in 8.3 > >>>if there is demand. > >>> > >>OK, I will put it back as it was in v8 > >>keeping all your other cleanup and > >>let Bruce and Tom decide. > > > >Hum, are you going to put back the original cruft to support copy view? > >I suggest you don't do that. > > Well, the other way around is to teach heap_open() > to use views. Brrr. Would it be any cleaner? Certainly not. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Zoltan Boszormenyi wrote:
>> My v8 had the syntax support for
>> COPY (SELECT ...) (col1, col2, ...) TO
>> and it was actually working. In your v9
>> you rewrote the syntax parsing so that
>> feature was lost in translation.
> Interesting. I didn't realize this was possible -- obviously I didn't
> test it (did you have a test for it in the regression tests? I may have
> missed it). In fact, I deliberately removed the column list from the
> grammar, because it can certainly be controlled inside the SELECT, so I
> thought there was no reason the support controlling it in the COPY
> column list.
I would vote against allowing a column list here, because it's useless
and it strikes me as likely to result in strange syntax error messages
if the user makes any little mistake. What worries me is that the
above looks way too nearly like a function call, which means that for
instance if you omit a right paren somewhere in the SELECT part, you're
likely to get a syntax error that points far to the right of the actual
mistake. The parser could also mistake the column list for a table-alias
column list.
Specifying a column list with a view name is useful, of course, but
what is the point when you are writing out a SELECT anyway?
regards, tom lane
Zoltan Boszormenyi <zboszor@dunaweb.hu> writes:
> Alvaro Herrera �rta:
>> Hum, are you going to put back the original cruft to support copy view?
>> I suggest you don't do that.
> Well, the other way around is to teach heap_open()
> to use views. Brrr. Would it be any cleaner?
Don't even think of going there ;-)
regards, tom lane
Tom Lane írta: > Zoltan Boszormenyi <zboszor@dunaweb.hu> writes: > >> Alvaro Herrera írta: >> >>> Hum, are you going to put back the original cruft to support copy view? >>> I suggest you don't do that. >>> > > >> Well, the other way around is to teach heap_open() >> to use views. Brrr. Would it be any cleaner? >> > > Don't even think of going there ;-) > > regards, tom lane > I didn't. :-) Here's my last, the "cruft" (i.e. COPY view TO support by rewriting to a SELECT) put back. Tested and docs modified accordingly. You can find the previous one (v10) on the list without it if you need it. Best regards, Zoltán Böszörményi
Attachment
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Zoltan Boszormenyi wrote: > >> My v8 had the syntax support for > >> COPY (SELECT ...) (col1, col2, ...) TO > >> and it was actually working. In your v9 > >> you rewrote the syntax parsing so that > >> feature was lost in translation. > > > Interesting. I didn't realize this was possible -- obviously I didn't > > test it (did you have a test for it in the regression tests? I may have > > missed it). In fact, I deliberately removed the column list from the > > grammar, because it can certainly be controlled inside the SELECT, so I > > thought there was no reason the support controlling it in the COPY > > column list. > > I would vote against allowing a column list here, because it's useless > and it strikes me as likely to result in strange syntax error messages > if the user makes any little mistake. What worries me is that the > above looks way too nearly like a function call, which means that for > instance if you omit a right paren somewhere in the SELECT part, you're > likely to get a syntax error that points far to the right of the actual > mistake. The parser could also mistake the column list for a table-alias > column list. > > Specifying a column list with a view name is useful, of course, but > what is the point when you are writing out a SELECT anyway? If you don't support COPY view TO, at least return an error messsage that suggests using COPY (SELECT * FROM view). And if you support COPY VIEW, you are going to have to support a column list for that. Is that additional complexity in COPY? If so, it might be a reason to just throw an error on views and do use COPY SELECT. Seeing that COPY VIEW only supports TO, not FROM, and COPY SELECT support only TO, not FROM, it seems logical for COPY to just support relations, and COPY SELECT to be used for views, if we can throw an error on COPY VIEW to tell people to use COPY SELECT. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Mon, Aug 28, 2006 at 19:35:11 +0200, Zoltan Boszormenyi <zboszor@dunaweb.hu> wrote: > > (BTW, is there anyone as high-ranking as them, > or the "committee" is a duumvirate? :-) ) There is a group referred to as "core" that is the final arbitrator of things. Tom and Bruce are both members of this group. Tom and Bruce tend to be the most visibly active "committers" for getting patches committed for people that can't do it themselves. So you will see them speak up more than others on the patches list.
Bruno Wolff III wrote: > On Mon, Aug 28, 2006 at 19:35:11 +0200, > Zoltan Boszormenyi <zboszor@dunaweb.hu> wrote: > > > > (BTW, is there anyone as high-ranking as them, > > or the "committee" is a duumvirate? :-) ) > > There is a group referred to as "core" that is the final arbitrator of things. > Tom and Bruce are both members of this group. > Tom and Bruce tend to be the most visibly active "committers" for getting > patches committed for people that can't do it themselves. So you will see them > speak up more than others on the patches list. We do try not to be decision-makers, but rather give our opinions and see how the group decides. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian írta: > Tom Lane wrote: > >> Alvaro Herrera <alvherre@commandprompt.com> writes: >> >>> Zoltan Boszormenyi wrote: >>> >>>> My v8 had the syntax support for >>>> COPY (SELECT ...) (col1, col2, ...) TO >>>> and it was actually working. In your v9 >>>> you rewrote the syntax parsing so that >>>> feature was lost in translation. >>>> >>> Interesting. I didn't realize this was possible -- obviously I didn't >>> test it (did you have a test for it in the regression tests? I may have >>> missed it). In fact, I deliberately removed the column list from the >>> grammar, because it can certainly be controlled inside the SELECT, so I >>> thought there was no reason the support controlling it in the COPY >>> column list. >>> >> I would vote against allowing a column list here, because it's useless >> and it strikes me as likely to result in strange syntax error messages >> if the user makes any little mistake. What worries me is that the >> above looks way too nearly like a function call, which means that for >> instance if you omit a right paren somewhere in the SELECT part, you're >> likely to get a syntax error that points far to the right of the actual >> mistake. The parser could also mistake the column list for a table-alias >> column list. >> >> Specifying a column list with a view name is useful, of course, but >> what is the point when you are writing out a SELECT anyway? >> > > If you don't support COPY view TO, at least return an error messsage > that suggests using COPY (SELECT * FROM view). And if you support COPY > VIEW, you are going to have to support a column list for that. Is that > additional complexity in COPY? If so, it might be a reason to just > throw an error on views and do use COPY SELECT. > No, it oes not have any additional complexity, it uses the same code COPY tablename TO uses. > Seeing that COPY VIEW only supports TO, not FROM, and COPY SELECT > support only TO, not FROM, it seems logical for COPY to just support > relations, and COPY SELECT to be used for views, if we can throw an > error on COPY VIEW to tell people to use COPY SELECT. > The additional hint would be enough if the VIEW case is not supported.
Hi, as per your suggestion, the COPY view TO support was cut and a hint was added. Please, review. Best regards, Zoltán Böszörményi
Attachment
On Mon, Aug 28, 2006 at 07:35:11PM +0200, Zoltan Boszormenyi wrote: > >>COPY (SELECT ...) (col1, col2, ...) TO > >>and it was actually working. In your v9 > >>you rewrote the syntax parsing so that > >>feature was lost in translation. > >> > > > >Interesting. I didn't realize this was possible -- obviously I didn't > >test it (did you have a test for it in the regression tests? I may have > >missed it). In fact, I deliberately removed the column list from the > >grammar, because it can certainly be controlled inside the SELECT, so I > >thought there was no reason the support controlling it in the COPY > >column list. > > > > Yes, it was even documented. I thought about having > queries stored statically somewhere (not in views) and > being able to use only part of the result. ISTM that there should have been a regression test that tried that capability out. That would have made it obvious when the functionality was lost, at least. -- Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com Pervasive Software http://pervasive.com work: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461
=?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <zboszor@dunaweb.hu> writes:
> as per your suggestion, the COPY view TO support was cut and
> a hint was added. Please, review.
Committed after some refactoring to avoid code duplication.
Unfortunately, in a moment of pure brain fade, I looked at the wrong
item in my inbox and wrote Bernd Helmle's name instead of yours in the
commit message :-(. My sincere apologies. Bruce, would you make a note
to be sure the right person gets credit in the release notes?
regards, tom lane
Tom Lane wrote: > =?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <zboszor@dunaweb.hu> writes: > > as per your suggestion, the COPY view TO support was cut and > > a hint was added. Please, review. > > Committed after some refactoring to avoid code duplication. > > Unfortunately, in a moment of pure brain fade, I looked at the wrong > item in my inbox and wrote Bernd Helmle's name instead of yours in the > commit message :-(. My sincere apologies. Bruce, would you make a note > to be sure the right person gets credit in the release notes? Fixed with new commit message to copy.c. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Thanks!!! Tom Lane írta: > =?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <zboszor@dunaweb.hu> writes: > >> as per your suggestion, the COPY view TO support was cut and >> a hint was added. Please, review. >> > > Committed after some refactoring to avoid code duplication. > > Unfortunately, in a moment of pure brain fade, I looked at the wrong > item in my inbox and wrote Bernd Helmle's name instead of yours in the > commit message :-(. My sincere apologies. Bruce, would you make a note > to be sure the right person gets credit in the release notes? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq > >