Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Thanks!!! Tom Lane írta: =?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
=?iso-8859-2?Q?B=F6sz=F6rm=E9nyi_Zolt=E1n?= <[EMAIL PROTECTED]> 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 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 [EMAIL PROTECTED] Pervasive Software http://pervasive.comwork: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 pgsql-copyselect-12.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
On Mon, Aug 28, 2006 at 19:35:11 +0200, Zoltan Boszormenyi <[EMAIL PROTECTED]> 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. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Tom Lane írta: Zoltan Boszormenyi <[EMAIL PROTECTED]> 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 pgsql-copyselect-11.patch.gz Description: Unix tar archive ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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? ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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? :-) )
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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")?
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> 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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera <[EMAIL PROTECTED]> 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
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 Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [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 pgsql-copyselect-10.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera <[EMAIL PROTECTED]> writes: > ... I'd do it myself but I'm heading to bed right now. > ... > I'll repost a reworked version at some point, if no one beats me to it. I was planning to start looking at this patch tomorrow (unless Gavin produces a new bitmap-index patch by then). I'll work from this one unless somebody produces a better version meanwhile. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Removed Cc: to pgsql-hackers.
Zoltán,
Zoltan Boszormenyi wrote:
> >Your patch has been added to the PostgreSQL unapplied patches list at:
> >
> > http://momjian.postgresql.org/cgi-bin/pgpatches
> >
> >It will be applied as soon as one of the PostgreSQL committers reviews
> >and approves it.
>
> Thanks. Would you please add this instead?
> psql built-in \copy (select ...) now also work.
Please check this one out. I took the version you posted here and
changed the stuff in the parser that I didn't like, and removed the ugly
"SELECT * FROM" stuff that was bothering me. I also removed the
transformCopyStmt stuff as it seems unnecessary to me. I did all that
stuff in a cleaner way (IMO).
I also cleaned up the grammar -- basically added a separate case from
the regular COPY. I took the opportunity to remove the
backwards-compatible options from there. I didn't check that stuff very
much but it should continue to work ...
I noticed that this works:
alvherre=# copy (values (1, 'uno'), (2, 'dos'), (3, 'tr;es'), (4, NULL)) to
stdout with delimiter ';' null 'NUL' csv quote as '"';
1;uno
2;dos
3;"tr;es"
4;NUL
which is nice.
With this patch, the COPY view FROM stdout path now throws an error --
in your version it worked (because of that "COPY * FROM" stuff), and
from previous discussion it seems reasonable to behave differently for
views than for plain tables (i.e. it's reasonable that we fail for
views).
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.
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
but didn't got around to it.
I also noticed that the new copyselect regression test is not added to
the serial schedule.
I'll repost a reworked version at some point, if no one beats me to it.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: doc/src/sgml/ref/copy.sgml
===
RCS file: /home/alvherre/cvs/pgsql/doc/src/sgml/ref/copy.sgml,v
retrieving revision 1.74
diff -c -p -r1.74 copy.sgml
*** doc/src/sgml/ref/copy.sgml 22 Apr 2006 03:03:11 - 1.74
--- doc/src/sgml/ref/copy.sgml 27 Aug 2006 04:53:19 -
***
*** 1,5
--- 1,5
*** COPY tabl
*** 33,39
[ ESCAPE [ AS ] 'escape' ]
[ FORCE NOT NULL column [, ...] ]
! COPY tablename [ ( column [, ...] ) ]
TO { 'filename' | STDOUT }
[ [ WITH ]
[ BINARY ]
--- 33,39
[ ESCAPE [ AS ] 'escape' ]
[ FORCE NOT NULL column [, ...] ]
! COPY { tablename | viewname | ( select_statement ) } [ (
column [, ...] ) ]
TO { 'filename' | STDOUT }
[ [ WITH ]
[ BINARY ]
*** COPY tabl
*** 55,61
COPY moves data between
PostgreSQL tables and standard file-system
files. COPY TO copies the contents of a table
!to a file, while COPY FROM copies
data from a file to a table (appending the data to
whatever is in the table already).
--- 55,63
COPY moves data between
PostgreSQL tables and standard file-system
files. COPY TO copies the contents of a table
!to a file, which also work on views and arbitrary
!SELECT statements. (Internally, the view case is rewitten as
!SELECT * FROM viewname.) COPY FROM
copies
data from a file to a table (appending the data to
whatever is in the table already).
*** COPY tabl
*** 65,71
only copy the data in the specified columns to or from the file.
If there are any columns in the table that are not in the column list,
COPY FROM will insert the default values for
!those columns.
--- 67,76
only copy the data in the specified columns to or from the file.
If there are any columns in the table that are not in the column list,
COPY FROM will insert the default values for
!those columns. COPY TO also accepts the list of
!columns, which can be useful for exporting only a subset of the
!columns that are in the table, view or select statement, or
!reordering them in the export.
*** COPY tabl
*** 148,154
Specifies copying the OID for each row. (An error is raised if
OIDS is specified for a table that does not
! have OIDs.)
--- 153,159
Specifies copying the OID for each row. (An error is raised if
OIDS is specified for a table that does not
! have OIDs, or in the case of COPY (SELECT) TO.)
Index: src/backend/commands/copy.c
===
RCS file:
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Bruce Momjian írta: Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. Thanks. Would you please add this instead? psql built-in \copy (select ...) now also work. Best regards, Zoltán Böszörményi pgsql-copyselect-8.patch.gz Description: Unix tar archive ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
> Böszörményi Zoltán <[EMAIL PROTECTED]> writes: >> With PostgreSQL 8.1.4, I used this: > >> begin; >> select ... into temp myquery1; >> copy myquery1 to stdout csv delimiter '|'; >> rollback; > > The performance of this would doubtless vary a lot with the temp_buffers > setting. Did you try different values? Yes, I did, but now checked back with 8.2CVS. The previously quoted result was achieved with temp_buffers = 1000 on both 8.1.4 and 8.2CVS. On 8.2CVS with temp_buffers = 4096, the 10 client case kills the machine with swapping, but the 3 client runtime with COPY(SELECT) went down to 2:41. The SELECT INTO TEMP case went down to 3:36. > It'd also be interesting to time the same way (with a temp table) in > devel. I don't remember whether we did any performance work on the > COPY CSV data path in this cycle, or whether that was all present in > 8.1. In any case it'd be worth proving that the COPY SELECT patch isn't > degrading performance of the copy-a-relation case. I will report back with that, say on Monday. In the meantime, I documented the COPY (SELECT) case and modified parser/analyze.c and tcop/utility.c so neither of them calls anything from under another directory. I think it's cleaner now. Also, I tried to implement more closely what printtup() does. Please, review. Best regards, Zoltán Böszörményi pgsql-copyselect-7.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
