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
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)
Bruce Momjian írta: Tom Lane wrote: 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? 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. ---(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
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
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
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
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
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
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
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
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
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
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 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
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
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
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
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
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
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
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
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
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: 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: 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: 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: 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 [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 [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
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)
Tom Lane wrote: 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? 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 [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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)
Bruno Wolff III wrote: 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. We do try not to be decision-makers, but rather give our opinions and see how the group decides. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
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
!--
! $PostgreSQL: pgsql/doc/src/sgml/ref/copy.sgml,v 1.73 2006/03/03 19:54:10 tgl
Exp $
PostgreSQL documentation
--
--- 1,5
!--
! $PostgreSQL: pgsql/doc/src/sgml/ref/copy.sgml,v 1.74 2006-04-22 03:03:11
momjian Exp $
PostgreSQL documentation
--
*** COPY replaceable class=parametertabl
*** 33,39
[ ESCAPE [ AS ] 'replaceable
class=parameterescape/replaceable' ]
[ FORCE NOT NULL replaceable
class=parametercolumn/replaceable [, ...] ]
! COPY replaceable class=parametertablename/replaceable [ ( replaceable
class=parametercolumn/replaceable [, ...] ) ]
TO { 'replaceable class=parameterfilename/replaceable' | STDOUT }
[ [ WITH ]
[ BINARY ]
--- 33,39
[ ESCAPE [ AS ] 'replaceable
class=parameterescape/replaceable' ]
[ FORCE NOT NULL replaceable
class=parametercolumn/replaceable [, ...] ]
! COPY { replaceable class=parametertablename/replaceable | replaceable
class=parameterviewname/replaceable | ( select_statement ) } [ (
replaceable class=parametercolumn/replaceable [, ...] ) ]
TO { 'replaceable class=parameterfilename/replaceable' | STDOUT }
[ [ WITH ]
[ BINARY ]
*** COPY replaceable class=parametertabl
*** 55,61
commandCOPY/command moves data between
productnamePostgreSQL/productname tables and standard file-system
files. commandCOPY TO/command copies the contents of a table
!emphasisto/ a file, while commandCOPY FROM/command copies
data emphasisfrom/ a file to a table (appending the data to
whatever is in the table already).
/para
--- 55,63
commandCOPY/command moves data between
productnamePostgreSQL/productname tables and standard file-system
files. commandCOPY TO/command copies the contents of a table
!emphasisto/ a file, which also work on views and arbitrary
!SELECT statements. (Internally, the view case is rewitten as
!commandSELECT * FROM viewname/command.) commandCOPY FROM/command
copies
data emphasisfrom/ a file to a table (appending the data to
whatever is in the table already).
/para
*** COPY replaceable class=parametertabl
*** 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,
commandCOPY FROM/command will insert the default values for
!those columns.
/para
para
--- 67,76
only copy the data in the
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
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
