Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi írta: Zoltan Boszormenyi írta: The option parsing and error checking is now common. I also changed it to use transformStmt() in analyze.c. However, both the UNION and sunselect cases give me something like this: ERROR: could not open relation 1663/16384/16723: No such file or directory What else can I do with it? But a single SELECT with two tables joined also works so it must be something trivial. Now UNIONs and subselects also work. Your concern about copy_dest_printtup() wasn't solved yet. Best regards, Zoltán Böszörményi pgsql-copyselect-5.patch.gz Description: Unix tar archive ---(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: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi írta: The option parsing and error checking is now common. I also changed it to use transformStmt() in analyze.c. However, both the UNION and sunselect cases give me something like this: ERROR: could not open relation 1663/16384/16723: No such file or directory What else can I do with it? But a single SELECT with two tables joined also works so it must be something trivial. 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: [HACKERS] [PATCHES] COPY view
Tom Lane írta: Zoltan Boszormenyi <[EMAIL PROTECTED]> writes: How about the callback solution for the SELECT case that was copied from the original? Should I consider open-coding in copy.c what ExecutorRun() does to avoid the callback? Adding a DestReceiver type is a good solution ... although that static variable is not. Instead define a DestReceiver extension struct that can carry the CopyState pointer for you. Done. You could also consider putting the copy-from-view-specific state fields into DestReceiver instead of CopyState, though this is a bit asymmetric with the relation case so maybe it's not really cleaner. Left it alone for now. BTW, lose the tuple_to_values function --- it's an extremely bad reimplementation of heap_deform_tuple. Done. copy_dest_printtup also seems coded without regard for the TupleTableSlot access API (read printtup() to see what to do instead). I am still interpreting it. Can you give me some hints besides using slot_getallattrs(slot)? And what's the point of factoring out the heap_getnext loop as CopyRelationTo? It's not like that lets you share any more code. The inside of the loop, ie what you've called CopyValuesTo, is the sharable part. Done. The option parsing and error checking is now common. I also changed it to use transformStmt() in analyze.c. However, both the UNION and sunselect cases give me something like this: ERROR: could not open relation 1663/16384/16723: No such file or directory What else can I do with it? Best regards, Zoltán Böszörményi pgsql-copyselect-4.patch.gz Description: Unix tar archive ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi <[EMAIL PROTECTED]> writes: > How about the callback solution for the SELECT case > that was copied from the original? Should I consider > open-coding in copy.c what ExecutorRun() does > to avoid the callback? Adding a DestReceiver type is a good solution ... although that static variable is not. Instead define a DestReceiver extension struct that can carry the CopyState pointer for you. You could also consider putting the copy-from-view-specific state fields into DestReceiver instead of CopyState, though this is a bit asymmetric with the relation case so maybe it's not really cleaner. BTW, lose the tuple_to_values function --- it's an extremely bad reimplementation of heap_deform_tuple. copy_dest_printtup also seems coded without regard for the TupleTableSlot access API (read printtup() to see what to do instead). And what's the point of factoring out the heap_getnext loop as CopyRelationTo? It's not like that lets you share any more code. The inside of the loop, ie what you've called CopyValuesTo, is the sharable part. regards, tom lane ---(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: [HACKERS] [PATCHES] COPY view
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Which is why you should leave the relation case alone and only add the > different case. The relation case is already known to be good. Well, a certain amount of refactoring of the code is inevitable unless we want a lot of code duplication. But I don't see a reason to do anything more to the relation-case code path than push some chunks of it into subroutines. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi wrote: > 1. to minimize the already huge code impact on the relation case. > 2. the checks done for the SELECT case is not exactly the > same as for the realation case. So put them in CopyToRelation. But the ones that apply to both, leave in CopyTo. > 3. the relation case is managed by passing around >a Relation pointer, e.g. CopyGetAttnums. This simply >not appropriate for the SELECT case. Which is why you should leave the relation case alone and only add the different case. The relation case is already known to be good. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Tom Lane írta: [ cc list trimmed to something reasonable ] Zoltan Boszormenyi <[EMAIL PROTECTED]> writes: OK, here's my current version. The reference leak is fixed. But as my testcase shows, it only works for single selects currently. The parser accepts it but COPY doesn't produce the expected output. Please, suggest a solution. This patch seems to be adding a tremendous amount of duplicated code in copy.c. Why? 1. to minimize the already huge code impact on the relation case. 2. the checks done for the SELECT case is not exactly the same as for the realation case. 3. the relation case is managed by passing around a Relation pointer, e.g. CopyGetAttnums. This simply not appropriate for the SELECT case. I will try to clean it up as much as I can, though. How about the callback solution for the SELECT case that was copied from the original? Should I consider open-coding in copy.c what ExecutorRun() does to avoid the callback? Also, moving checks for the relation case out of copy.c and into analyze.c is inappropriate. The checks you have moved there are actually wrong because you have no lock on the relation at the time you are checking. You could perhaps take a lock at analyze time, but frankly I see no reason for this patch to be messing with the relation case at all. OK, I will put the checks back where they were. As for the UNION problem, try passing the query to transformStmt rather than prejudging where transformStmt will send it. Compare for instance the analyze.c code for ExplainStmt. Thanks. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: frankly I see no reason for this patch to be messing with the relation case at all. Quite apart from anything else, if it's done that way nothing that currently works gets broken. cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi wrote: > The exact same code was there, > e.g. parse and rewrite "SELECT * FROM view" > just not in analyze.c. I will try without it, though. And it was wrong as well. (The code was there on the COPY-view patch, not on the official code). -- 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: [HACKERS] [PATCHES] COPY view
[ cc list trimmed to something reasonable ] Zoltan Boszormenyi <[EMAIL PROTECTED]> writes: > OK, here's my current version. The reference leak is fixed. > But as my testcase shows, it only works for single selects > currently. The parser accepts it but COPY doesn't produce > the expected output. Please, suggest a solution. This patch seems to be adding a tremendous amount of duplicated code in copy.c. Why? Also, moving checks for the relation case out of copy.c and into analyze.c is inappropriate. The checks you have moved there are actually wrong because you have no lock on the relation at the time you are checking. You could perhaps take a lock at analyze time, but frankly I see no reason for this patch to be messing with the relation case at all. As for the UNION problem, try passing the query to transformStmt rather than prejudging where transformStmt will send it. Compare for instance the analyze.c code for ExplainStmt. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] COPY view
Alvaro Herrera írta: Zoltan Boszormenyi wrote: OK, here's my current version. The reference leak is fixed. But as my testcase shows, it only works for single selects currently. The parser accepts it but COPY doesn't produce the expected output. Please, suggest a solution. I'm not sure I agree with the approach of creating a fake "SELECT * FROM foo" in analyze.c in the relation case and passing it back to the parser to create a Query node. That's not there in the original code and you shouldn't need it. Just let the case where COPY gets a relation continue to handle it as it does today, and add a separate case for the SELECT. The exact same code was there, e.g. parse and rewrite "SELECT * FROM view" just not in analyze.c. I will try without it, though. That doesn't help you with the UNION stuff though. :-( ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi wrote: > OK, here's my current version. The reference leak is fixed. > But as my testcase shows, it only works for single selects > currently. The parser accepts it but COPY doesn't produce > the expected output. Please, suggest a solution. I'm not sure I agree with the approach of creating a fake "SELECT * FROM foo" in analyze.c in the relation case and passing it back to the parser to create a Query node. That's not there in the original code and you shouldn't need it. Just let the case where COPY gets a relation continue to handle it as it does today, and add a separate case for the SELECT. That doesn't help you with the UNION stuff though. -- 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: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi írta: Hi, Bruce Momjian írta: Alvaro Herrera wrote: Bruce Momjian wrote: Andrew Dunstan wrote: Bruce Momjian wrote: I think Alvaro is saying we need it in a few days, not longer. I thought he was saying today ;-) He actually said "now", but I don't think we need it immediately, especially if he is still working on it. We are at least 1-2 weeks away from having all open patches applied. Yes, I'm saying today so that we can all look at it and point obvious mistakes now, not in 2 weeks from now. Release early, release often. If the patch contains a mistake and we find out in 2 weeks, are we going to fix it? No, we are going to reject it. OK, I understand. B?sz?rm?nyi, post now so we can see where you are, but keep working and send it to us again when you are done. No sense in not posting your working version. OK, here's my current version. The reference leak is fixed. But as my testcase shows, it only works for single selects currently. The parser accepts it but COPY doesn't produce the expected output. Please, suggest a solution. I meant that UNION selects, subselects don't work yet. BTW, my first name is Zoltán. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq ---(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: [HACKERS] [PATCHES] COPY view
Hi, Bruce Momjian írta: Alvaro Herrera wrote: Bruce Momjian wrote: Andrew Dunstan wrote: Bruce Momjian wrote: I think Alvaro is saying we need it in a few days, not longer. I thought he was saying today ;-) He actually said "now", but I don't think we need it immediately, especially if he is still working on it. We are at least 1-2 weeks away from having all open patches applied. Yes, I'm saying today so that we can all look at it and point obvious mistakes now, not in 2 weeks from now. Release early, release often. If the patch contains a mistake and we find out in 2 weeks, are we going to fix it? No, we are going to reject it. OK, I understand. B?sz?rm?nyi, post now so we can see where you are, but keep working and send it to us again when you are done. No sense in not posting your working version. OK, here's my current version. The reference leak is fixed. But as my testcase shows, it only works for single selects currently. The parser accepts it but COPY doesn't produce the expected output. Please, suggest a solution. BTW, my first name is Zoltán. Best regards, Zoltán Böszörményi pgsql-copyselect.patch.gz Description: Unix tar archive ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
Alvaro Herrera wrote: > Bruce Momjian wrote: > > Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > > > I think Alvaro is saying we need it in a few days, not longer. > > > > > > I thought he was saying today ;-) > > > > He actually said "now", but I don't think we need it immediately, > > especially if he is still working on it. We are at least 1-2 weeks away > > from having all open patches applied. > > Yes, I'm saying today so that we can all look at it and point obvious > mistakes now, not in 2 weeks from now. Release early, release often. > If the patch contains a mistake and we find out in 2 weeks, are we going > to fix it? No, we are going to reject it. OK, I understand. B?sz?rm?nyi, post now so we can see where you are, but keep working and send it to us again when you are done. No sense in not posting your working version. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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: [HACKERS] [PATCHES] COPY view
Bruce Momjian wrote: > Andrew Dunstan wrote: > > Bruce Momjian wrote: > > > I think Alvaro is saying we need it in a few days, not longer. > > > > I thought he was saying today ;-) > > He actually said "now", but I don't think we need it immediately, > especially if he is still working on it. We are at least 1-2 weeks away > from having all open patches applied. Yes, I'm saying today so that we can all look at it and point obvious mistakes now, not in 2 weeks from now. Release early, release often. If the patch contains a mistake and we find out in 2 weeks, are we going to fix it? No, we are going to reject it. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Andrew Dunstan wrote: > Bruce Momjian wrote: > > I think Alvaro is saying we need it in a few days, not longer. > > > > > > I thought he was saying today ;-) He actually said "now", but I don't think we need it immediately, especially if he is still working on it. We are at least 1-2 weeks away from having all open patches applied. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Bruce Momjian wrote: I think Alvaro is saying we need it in a few days, not longer. I thought he was saying today ;-) cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
> Böszörményi Zoltán wrote: >> > B?sz?rm?nyi Zolt?n wrote: >> > >> >> > So when will you send in a revised patch? >> >> >> >> Soon. :-) >> > >> > No, don't send it "soon". We're in feature freeze already (and have >> > been for three weeks). You need to send it now. >> >> I have to test it some more but I will send it. > > I think Alvaro is saying we need it in a few days, not longer. Of course. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
B?sz?rm?nyi Zolt?n wrote: > > B?sz?rm?nyi Zolt?n wrote: > > > >> > So when will you send in a revised patch? > >> > >> Soon. :-) > > > > No, don't send it "soon". We're in feature freeze already (and have > > been for three weeks). You need to send it now. > > I have to test it some more but I will send it. I think Alvaro is saying we need it in a few days, not longer. -- 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: [HACKERS] [PATCHES] COPY view
> Böszörményi Zoltán wrote: > >> > So when will you send in a revised patch? >> >> Soon. :-) > > No, don't send it "soon". We're in feature freeze already (and have > been for three weeks). You need to send it now. I have to test it some more but I will send it. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Böszörményi Zoltán wrote: > > So when will you send in a revised patch? > > Soon. :-) No, don't send it "soon". We're in feature freeze already (and have been for three weeks). You need to send it now. -- 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: [HACKERS] [PATCHES] COPY view
Böszörményi Zoltán wrote: > It seems I was able to get it working for both the VIEW and SELECT > cases. I still have one issue, the reference to the select is left open > and it complains on closing the transaction. But basically works. Cool, thanks. Send the patch and we can look it over to see what you're missing. It may be something simple like shutting down the executor node or something. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
> Böszörményi Zoltán wrote: >> Hi, >> >> >>> Tom Lane wrote: >>> At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. >>> Personally, I would say that this is less important than updatable >>> views >>> but more than online indexes. If it could be fixed just for the view >>> case in a day or so then I think it's worth it. >>> >>> cheers >>> >>> andrew >>> >> >> It seems I was able to get it working for both the VIEW and SELECT >> cases. I still have one issue, the reference to the select is left open >> and it complains on closing the transaction. But basically works. >> >> >> > > So when will you send in a revised patch? > > cheers > > andrew Soon. :-) 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: [HACKERS] [PATCHES] COPY view
Böszörményi Zoltán wrote: Hi, Tom Lane wrote: At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. Personally, I would say that this is less important than updatable views but more than online indexes. If it could be fixed just for the view case in a day or so then I think it's worth it. cheers andrew It seems I was able to get it working for both the VIEW and SELECT cases. I still have one issue, the reference to the select is left open and it complains on closing the transaction. But basically works. So when will you send in a revised patch? cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Hi, > Tom Lane wrote: >> >> At the moment, with the online-index and updatable-views patches both >> pretty seriously broken, and no sign that the bitmap-index people are >> awake at all, I might take it on myself to fix this one instead of those >> others. But is that what I should be spending my time on in the waning >> days of the 8.2 freeze cycle? Speak now or hold your peace. >> >> >> > > Personally, I would say that this is less important than updatable views > but more than online indexes. If it could be fixed just for the view > case in a day or so then I think it's worth it. > > cheers > > andrew It seems I was able to get it working for both the VIEW and SELECT cases. I still have one issue, the reference to the select is left open and it complains on closing the transaction. But basically works. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
--On Mittwoch, August 23, 2006 08:24:55 -0400 Tom Lane <[EMAIL PROTECTED]> wrote: What are these open issues for the updatable views patch you are seeing exactly? Didn't Alvaro list a bunch of issues when he put the patch back up for comment? I have not looked at it myself yet. Indeed he did and this helps a lot to clean up some parts of the code (oh, thanks to him for reviewing this, i think i forgot that :( ). I thought you were refering to some specific showstoppers i've missed. -- Thanks Bernd ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Bernd Helmle <[EMAIL PROTECTED]> writes: > What are these open issues for the updatable views patch you are seeing > exactly? Didn't Alvaro list a bunch of issues when he put the patch back up for comment? I have not looked at it myself yet. > i see the INSERT...RETURNING stuff as the only "big hurd" at the moment That's not the fault of the updatable-views patch, but it definitely is something we need to put some time into :-( regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. Personally, I would say that this is less important than updatable views but more than online indexes. If it could be fixed just for the view case in a day or so then I think it's worth it. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Leaving... (was: Re: [HACKERS] [PATCHES] COPY view)
Hi all, seriously... I don't have time to work on PostgreSQL. It's time to say that I'm leaving this project. So, if you found some my broken code or whatever in PostgreSQL you should go and fix it. It's community-driven project. It's about collaboration -- don't ask "why should I help" -- go and help! It was nice time and really big experience, but in the world is more projects and many of them need more help than already stable (do you remember PostgreSQL 6.5? :-) and very reliable PostgreSQL. Good bye! Karel On Tue, Aug 22, 2006 at 11:12:21PM -0400, Tom Lane wrote: > The patch submitter has neither provided an updated patch nor defended > his original submission as being the right thing. If he doesn't take it > seriously enough to have done any followup, why should the rest of us? -- Karel Zak <[EMAIL PROTECTED]> ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
On Tue, Aug 22, 2006 at 01:11:22PM -0400, Andrew Dunstan wrote: > There's nothing hidden (unless it's also hidden from me ;-) ) > > I take it that when you talk about "we did this" you are referring to > the patch from Karel Zak. Hans has been original author of COPY VIEW idea and I've wrote it for his customer (yes, it was sponsored work). Karel -- Karel Zak <[EMAIL PROTECTED]> ---(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: [HACKERS] [PATCHES] COPY view
--On Dienstag, August 22, 2006 23:12:21 -0400 Tom Lane <[EMAIL PROTECTED]> wrote: At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. What are these open issues for the updatable views patch you are seeing exactly? I'm currently trying to update this patch based on alvaros comments in the code and i see the INSERT...RETURNING stuff as the only "big hurd" at the moment (however, i haven't looked at this closer, but saw your and Jaime's comments on this...). It would be nice if we could summarize all open things so everybody who is able to work on this gets a complete overview. -- Thanks Bernd ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Hi, > Robert Treat <[EMAIL PROTECTED]> writes: >> On Tuesday 22 August 2006 16:10, Tom Lane wrote: >>> As I see it, we've effectively got a patch that was rejected once, >>> and Bruce wants to apply it anyway because no replacement has been >>> forthcoming. > >> Well, unless someone is going to commit to doing it the other way, it >> seems >> the guy who actually codes something offers a better solution than >> handwaving... people have also had plenty of time to come up with a >> replacement if that's what they really wanted. > > The patch submitter has neither provided an updated patch nor defended > his original submission as being the right thing. If he doesn't take it > seriously enough to have done any followup, why should the rest of us? > > At the moment, with the online-index and updatable-views patches both > pretty seriously broken, and no sign that the bitmap-index people are > awake at all, I might take it on myself to fix this one instead of those > others. But is that what I should be spending my time on in the waning > days of the 8.2 freeze cycle? Speak now or hold your peace. > > regards, tom lane I am willing to get it up to shape and support both COPY (select) TO and COPY view TO, the second is rewritten as SELECT * FROM view. In fact, I already started. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: > Robert Treat <[EMAIL PROTECTED]> writes: > > On Tuesday 22 August 2006 16:10, Tom Lane wrote: > >> As I see it, we've effectively got a patch that was rejected once, > >> and Bruce wants to apply it anyway because no replacement has been > >> forthcoming. > > > Well, unless someone is going to commit to doing it the other way, it seems > > the guy who actually codes something offers a better solution than > > handwaving... people have also had plenty of time to come up with a > > replacement if that's what they really wanted. > > The patch submitter has neither provided an updated patch nor defended > his original submission as being the right thing. If he doesn't take it > seriously enough to have done any followup, why should the rest of us? > > At the moment, with the online-index and updatable-views patches both > pretty seriously broken, and no sign that the bitmap-index people are > awake at all, I might take it on myself to fix this one instead of those > others. But is that what I should be spending my time on in the waning > days of the 8.2 freeze cycle? Speak now or hold your peace. Your analysis is accurate. You can spend your time on whatever _you_ think is important. If someone wants to take on COPY VIEW and do all the work to make it 100%, then they are welcome to do it, but if you don't feel it is worth it, you can just leave it. If it isn't 100% by the time we start beta, it is kept for a later release. Alvaro has already indicated some problems with the patch (the objection email is in the patches queue), so it is up to someone to correct at least that, and if other objections are found, they have to correct those too before 8.2 beta starts. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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: [HACKERS] [PATCHES] COPY view
Robert Treat <[EMAIL PROTECTED]> writes: > On Tuesday 22 August 2006 16:10, Tom Lane wrote: >> As I see it, we've effectively got a patch that was rejected once, >> and Bruce wants to apply it anyway because no replacement has been >> forthcoming. > Well, unless someone is going to commit to doing it the other way, it seems > the guy who actually codes something offers a better solution than > handwaving... people have also had plenty of time to come up with a > replacement if that's what they really wanted. The patch submitter has neither provided an updated patch nor defended his original submission as being the right thing. If he doesn't take it seriously enough to have done any followup, why should the rest of us? At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] COPY view
On Tuesday 22 August 2006 16:10, Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > It sucks that patches are posted and no action is taken on them for
> > months. I agree with that.
>
> This particular patch was originally posted during the 8.1 feature
> freeze window (2005-09-29), so it was doomed to a certain amount of
> languishing on the to-worry-about-later list in any case. We should
> have gotten around to reviewing it sooner than we did (the followup
> discussion was around 2006-06-14), but there was still plenty of time
> at that point to rework it per the discussion and get it into 8.2.
>
> As I see it, we've effectively got a patch that was rejected once,
> and Bruce wants to apply it anyway because no replacement has been
> forthcoming.
>
Well, unless someone is going to commit to doing it the other way, it seems
the guy who actually codes something offers a better solution than
handwaving... people have also had plenty of time to come up with a
replacement if that's what they really wanted.
--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > It sucks that patches are posted and no action is taken on them for > > months. I agree with that. > > This particular patch was originally posted during the 8.1 feature > freeze window (2005-09-29), so it was doomed to a certain amount of > languishing on the to-worry-about-later list in any case. We should > have gotten around to reviewing it sooner than we did (the followup > discussion was around 2006-06-14), but there was still plenty of time > at that point to rework it per the discussion and get it into 8.2. > > As I see it, we've effectively got a patch that was rejected once, > and Bruce wants to apply it anyway because no replacement has been > forthcoming. Yea, that pretty much sums it up. Based on the number of people who wanted it applied, I think we need to have a discussion like this. I can easily go with rejecting it, but I think the discussion is needed to be fair to the patch author. So, what do we want to do with this? Where did we say we didn't want SELECT? I never remember that being discussed. I remember us saying we never wanted SELECT or VIEWs because it was going to be slow, but once the SELECT idea came up, I think we decided we wanted that, and views could be built on top of that. I certainly never remember us saying we didn't want SELECT but wanted views. -- 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: [HACKERS] [PATCHES] COPY view
Alvaro Herrera <[EMAIL PROTECTED]> writes: > It sucks that patches are posted and no action is taken on them for > months. I agree with that. This particular patch was originally posted during the 8.1 feature freeze window (2005-09-29), so it was doomed to a certain amount of languishing on the to-worry-about-later list in any case. We should have gotten around to reviewing it sooner than we did (the followup discussion was around 2006-06-14), but there was still plenty of time at that point to rework it per the discussion and get it into 8.2. As I see it, we've effectively got a patch that was rejected once, and Bruce wants to apply it anyway because no replacement has been forthcoming. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Hans-Juergen Schoenig wrote: > It has been made as "COPY FROM / TO view" because people wanted it to be > done that way. > My original proposal was in favour of arbitrary SELECTs (just like > proposed by the TODO list) but this was rejected. So, we did it that way > (had to explain to customer why views are better). Now everybody wants > the original select which was proposed. This is not the first time this happens. It has happened to Simon Riggs at least once and to me as well. Sometimes "the community" just doesn't realize what it wants, until what it think it wants is done and then realizes it wants something else. It is frustrating, but I don't see how to do things differently. > Things have been submitted months ago and now we are short of time. I > think everybody on the list is going a superior job but after 6 years I > still have no idea how patches are treated ;). It sucks that patches are posted and no action is taken on them for months. I agree with that. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: Bruce Momjian wrote: Well, the patch was submitted in time, and it is a desired feature. If we want to hold it for 8.3 due to lack of time, we can, but I don't think we can decide now that it must wait. well I thought the agreed approach to that was allowing COPY from arbitrary expressions without the need to go through the extra CREATE VIEW step? Exactly. This is not the feature that was agreed to. Just because we have a patch for it doesn't mean that we have to put it in. If we do put it in, we'll be stuck carrying that feature forever, even after someone gets around to doing it right. regards, tom lane It has been made as "COPY FROM / TO view" because people wanted it to be done that way. My original proposal was in favour of arbitrary SELECTs (just like proposed by the TODO list) but this was rejected. So, we did it that way (had to explain to customer why views are better). Now everybody wants the original select which was proposed. I can understand if things are not committed because of bad code quality or whatever but to be honest: It is more of less frustrating if things are done differently because of community wish and then rejected because things are not done the original way ... Things have been submitted months ago and now we are short of time. I think everybody on the list is going a superior job but after 6 years I still have no idea how patches are treated ;). 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: [HACKERS] [PATCHES] COPY view
Hans-Juergen Schoenig wrote: Tom Lane wrote: Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: Bruce Momjian wrote: Well, the patch was submitted in time, and it is a desired feature. If we want to hold it for 8.3 due to lack of time, we can, but I don't think we can decide now that it must wait. well I thought the agreed approach to that was allowing COPY from arbitrary expressions without the need to go through the extra CREATE VIEW step? Exactly. This is not the feature that was agreed to. Just because we have a patch for it doesn't mean that we have to put it in. If we do put it in, we'll be stuck carrying that feature forever, even after someone gets around to doing it right. regards, tom lane It has been made as "COPY FROM / TO view" because people wanted it to be done that way. My original proposal was in favour of arbitrary SELECTs (just like proposed by the TODO list) but this was rejected. So, we did it that way (had to explain to customer why views are better). Now everybody wants the original select which was proposed. I can understand if things are not committed because of bad code quality or whatever but to be honest: It is more of less frustrating if things are done differently because of community wish and then rejected because things are not done the original way ... Things have been submitted months ago and now we are short of time. I think everybody on the list is going a superior job but after 6 years I still have no idea how patches are treated ;). There's nothing hidden (unless it's also hidden from me ;-) ) I take it that when you talk about "we did this" you are referring to the patch from Karel Zak. I have had a quick look at that patch, and apart from not applying cleanly to the current CVS tree (which isn't your fault as the patch has been sitting around for so long) it is also missing regression tests and docs. That's without even looking at code quality. So, how quickly can you fix those 3 things? 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: [HACKERS] [PATCHES] COPY view
Andrew Dunstan <[EMAIL PROTECTED]> writes: > It's a close call. On balance I'd be inclined to accept the patch if it > reviews OK, even though we will throw the code away soon (we hope). Well, the patch seems pretty ugly code-wise as well. I'd be willing to clean it up if I thought it wouldn't ultimately get yanked out again, but I'm not sure that I see the point if we think it's a dead end. It doesn't come close to applying to CVS HEAD, either :-( regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Stefan Kaltenbrunner wrote: Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian <[EMAIL PROTECTED]> writes: OK, based on this feedback, I am adding COPY VIEW to the patches queue. I think we have other things that demand our attention more than a half-baked feature. Well, the patch was submitted in time, and it is a desired feature. If we want to hold it for 8.3 due to lack of time, we can, but I don't think we can decide now that it must wait. well I thought the agreed approach to that was allowing COPY from arbitrary expressions without the need to go through the extra CREATE VIEW step? Well, it's been a bit of a mess, unfortunately, and I can understand people feeling aggrieved. I think there is general agreement that we want to be able to do: COPY (SELECT ... ) TO ... When we have that it would not be unreasonable to have a special case for views which would transparently rewrite COPY VIEWNAME TO as COPY (SELECT * FROM VIEWNAME) TO So we would not necessarily be adopting a feature we don't want in the long run, from a user visibility angle. The issue seems to be that in adopting the present patch we would be incorporating some code we will essentially have to abandon when we get the feature we all really want, and which we hope will be available for 8.3. On that basis I can certainly appreciate Tom's reluctance to adopt the patch. It's a close call. On balance I'd be inclined to accept the patch if it reviews OK, even though we will throw the code away soon (we hope). cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: > Bruce Momjian wrote: >> Well, the patch was submitted in time, and it is a desired feature. If >> we want to hold it for 8.3 due to lack of time, we can, but I don't >> think we can decide now that it must wait. > well I thought the agreed approach to that was allowing COPY from > arbitrary expressions without the need to go through the extra CREATE > VIEW step? Exactly. This is not the feature that was agreed to. Just because we have a patch for it doesn't mean that we have to put it in. If we do put it in, we'll be stuck carrying that feature forever, even after someone gets around to doing it right. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Bruce Momjian wrote: > Tom Lane wrote: >> Bruce Momjian <[EMAIL PROTECTED]> writes: >>> OK, based on this feedback, I am adding COPY VIEW to the patches queue. >> I think we have other things that demand our attention more than a >> half-baked feature. > > Well, the patch was submitted in time, and it is a desired feature. If > we want to hold it for 8.3 due to lack of time, we can, but I don't > think we can decide now that it must wait. well I thought the agreed approach to that was allowing COPY from arbitrary expressions without the need to go through the extra CREATE VIEW step? Stefan ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > OK, based on this feedback, I am adding COPY VIEW to the patches queue. > > I think we have other things that demand our attention more than a > half-baked feature. Well, the patch was submitted in time, and it is a desired feature. If we want to hold it for 8.3 due to lack of time, we can, but I don't think we can decide now that it must wait. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
Bruce Momjian <[EMAIL PROTECTED]> writes: > OK, based on this feedback, I am adding COPY VIEW to the patches queue. I think we have other things that demand our attention more than a half-baked feature. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
