Thread: Tablespace for temporary objects and sort files
Hi, I'm trying to introduce myself into postgresql development and I'm working on the "tablespace for temporary objects and sort files" TODO item. The attached patch shows what I've already done. The GUC is currently called "temp_tablespaces". The tablespace changes correctly for me when creating temporary tables. I've got some questions though: How can I test that the tablespace is correctly used for sort files? Is there an easy way? Or should I reduce work_mem to a minimum, populate the database with data and try an "ORDER BY"? The GetTempTablespace function correctly returns a different tablespace each time is called, but I store the position of the last tablespace used with an integer and iterate through the list of tablespaces each time. I tried to keep the iterator from call to call but I got a segfault, I imagine due to the memory context. Should I try to keep the iterator? How can I do it? Hope the diff and idents are ok. Please let me know if there's something wrong with them. Thanks!
Attachment
On Wed, 2006-10-25 at 00:45 +0200, Albert Cervera Areny wrote: > Hope the diff and idents are ok. Patches should be submitted in context diff (diff -c) format. -Neil
On 10/24/06, Albert Cervera Areny <albertca@hotpop.com> wrote: > Hi, > > I'm trying to introduce myself into postgresql development and I'm working on > the "tablespace for temporary objects and sort files" TODO item. some comments from a non-hacker: your patch isn't doing nothing at all for temporary indexes... a quick search for GetDefaultTablespace() shows this places... postgres@casanova:~/PG_RELEASES/pgsql$ grep -lR GetDefaultTablespace * src/backend/commands/indexcmds.c src/backend/commands/tablecmds.c src/backend/commands/tablespace.c src/backend/executor/execMain.c src/include/commands/tablespace.h Now a question, why not using the same GetDefaultTablespace() with an argument indicating if the object is temporary, if it is get the default tablespace for temp objects else get the default tablespace for permanent object... just an idea... > How can I test that the tablespace is correctly used for sort files? Is there > an easy way? Or should I reduce work_mem to a minimum, populate the database > with data and try an "ORDER BY"? > yes, that seems to work... i reduce, just in case, work_mem, shared_buffers and temp_buffers... Now, PG_TEMP_FILES_DIR seems to add just pgsql_temp to the filename. i think you the function you have to modify here is make_database_relative() that adds base/#database_oid# at the beginning of the path of the file. > > Hope the diff and idents are ok. Please let me know if there's something wrong > with them. > diff -c is the way -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Sorry about the wrong diff format. Attached is the good one. A Dimecres 25 Octubre 2006 09:07, Neil Conway va escriure: > On Wed, 2006-10-25 at 00:45 +0200, Albert Cervera Areny wrote: > > Hope the diff and idents are ok. > > Patches should be submitted in context diff (diff -c) format. > > -Neil
Attachment
This has been saved for the 8.3 release: http://momjian.postgresql.org/cgi-bin/pgpatches_hold --------------------------------------------------------------------------- Albert Cervera Areny wrote: > Sorry about the wrong diff format. Attached is the good one. > > A Dimecres 25 Octubre 2006 09:07, Neil Conway va escriure: > > On Wed, 2006-10-25 at 00:45 +0200, Albert Cervera Areny wrote: > > > Hope the diff and idents are ok. > > > > Patches should be submitted in context diff (diff -c) format. > > > > -Neil > [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
> -----Original Message----- > From: pgsql-patches-owner@postgresql.org > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Bruce Momjian > Sent: 26 October 2006 14:19 > To: Albert Cervera Areny > Cc: pgsql-patches@postgresql.org > Subject: Re: [PATCHES] Tablespace for temporary objects and sort files > > > This has been saved for the 8.3 release: > > http://momjian.postgresql.org/cgi-bin/pgpatches_hold > I believe Albert was looking for feedback at this stage, rather than submitting a completed patch. http://archives.postgresql.org/pgsql-patches/2006-10/msg00141.php Regards, Dave
bruce wrote: > Dave Page wrote: > > > > > > > -----Original Message----- > > > From: pgsql-patches-owner@postgresql.org > > > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Bruce Momjian > > > Sent: 26 October 2006 14:19 > > > To: Albert Cervera Areny > > > Cc: pgsql-patches@postgresql.org > > > Subject: Re: [PATCHES] Tablespace for temporary objects and sort files > > > > > > > > > This has been saved for the 8.3 release: > > > > > > http://momjian.postgresql.org/cgi-bin/pgpatches_hold > > > > > > > I believe Albert was looking for feedback at this stage, rather than > > submitting a completed patch. > > > > http://archives.postgresql.org/pgsql-patches/2006-10/msg00141.php > > Right, but it is being kept so that we can ping him when we start 8.3 to > get a more recent version. Sorry, to clarify, I saved the entire thread where he stated he was looking for feedback, so when we go back to it, we know it needs work. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
> -----Original Message----- > From: Bruce Momjian [mailto:bruce@momjian.us] > Sent: 26 October 2006 14:37 > To: bruce@momjian.us > Cc: Dave Page; Albert Cervera Areny; pgsql-patches@postgresql.org > Subject: Re: [PATCHES] Tablespace for temporary objects and sort files > > bruce wrote: > > Dave Page wrote: > > > > > > > > > > -----Original Message----- > > > > From: pgsql-patches-owner@postgresql.org > > > > [mailto:pgsql-patches-owner@postgresql.org] On Behalf > Of Bruce Momjian > > > > Sent: 26 October 2006 14:19 > > > > To: Albert Cervera Areny > > > > Cc: pgsql-patches@postgresql.org > > > > Subject: Re: [PATCHES] Tablespace for temporary objects > and sort files > > > > > > > > > > > > This has been saved for the 8.3 release: > > > > > > > > http://momjian.postgresql.org/cgi-bin/pgpatches_hold > > > > > > > > > > I believe Albert was looking for feedback at this stage, > rather than > > > submitting a completed patch. > > > > > > http://archives.postgresql.org/pgsql-patches/2006-10/msg00141.php > > > > Right, but it is being kept so that we can ping him when we > start 8.3 to > > get a more recent version. > > Sorry, to clarify, I saved the entire thread where he stated he was > looking for feedback, so when we go back to it, we know it needs work. OK. /D
Dave Page wrote: > > > > -----Original Message----- > > From: pgsql-patches-owner@postgresql.org > > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Bruce Momjian > > Sent: 26 October 2006 14:19 > > To: Albert Cervera Areny > > Cc: pgsql-patches@postgresql.org > > Subject: Re: [PATCHES] Tablespace for temporary objects and sort files > > > > > > This has been saved for the 8.3 release: > > > > http://momjian.postgresql.org/cgi-bin/pgpatches_hold > > > > I believe Albert was looking for feedback at this stage, rather than > submitting a completed patch. > > http://archives.postgresql.org/pgsql-patches/2006-10/msg00141.php Right, but it is being kept so that we can ping him when we start 8.3 to get a more recent version. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On 10/25/06, Jaime Casanova <systemguards@gmail.com> wrote: > On 10/24/06, Albert Cervera Areny <albertca@hotpop.com> wrote: > > I'm trying to introduce myself into postgresql development and I'm working on > > the "tablespace for temporary objects and sort files" TODO item. > > > Now, PG_TEMP_FILES_DIR seems to add just pgsql_temp to the filename. i > think you the function you have to modify here is > make_database_relative() that adds base/#database_oid# at the > beginning of the path of the file. > some tests shows that your patch is doing the wrong thing for temp files, actually the server is crashing -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
On 10/24/06, Albert Cervera Areny <albertca@hotpop.com> wrote: > Hi, > > I'm trying to introduce myself into postgresql development and I'm working on > the "tablespace for temporary objects and sort files" TODO item. The attached > patch shows what I've already done. The GUC is currently > called "temp_tablespaces". > hi albert. are you working on this? i'm willing to help... if you are not working on this, i will make a try... comments on the original patch? AFAIR, it fails the regrress tests when executing initdb... more here http://archives.postgresql.org/pgsql-patches/2006-10/msg00141.php -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
On 12/24/06, Albert Cervera Areny <albertca@hotpop.com> wrote: > Hi, > yes I'm working on it. I've got a more recent version that doesn't crash on > initdb and works with sort files. I've got a couple of things pending and > will post it as soon as I can. If you want I could make a diff and send it to > you in case you wanted to test/improve the patch. > Refering to some of the comment I never answered (sorry about that) I prefer > to use GetTempTablespace() as I think it's easier to read/understand than > GetDefaultTablespace(true). > And thanks for pointing to temporary indexes. There seems not to be temporary > indexes but indexes of temporary tables, which could use GetTempTablespace() > too... > yeah, it was late and i was almost asleep... i was thinking in temporary sequences, but indexes on temporary tables it's not a bad idea too... that was the reason i think it's better to use the same GetDefaultTablespace() function it's less intrussive and is not directed to one particular object but all temp objects can benefit... i will wait your patch when you think is ready for discussion... -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Hi, here's a new version of the patch against HEAD with both, table and sort files working correctly for me. Regression tests work too. I'd like to ask again the question I made on the first post as no answer was given at that time: "The GetTempTablespace function correctly returns a different tablespace each time is called, but I store the position of the last tablespace used with an integer and iterate through the list of tablespaces each time. I tried to keep the iterator from call to call but I got a segfault, I imagine due to the memory context. Should I try to keep the iterator? How can I do it?" Now I'm working on some regression tests that could be added to tablespace.source using something like: SET temp_tablespaces='testspace'; CREATE TEMP TABLE temp_foo(a VARCHAR); SELECT COUNT(pg_ls_dir('pg_tblspc/' || (SELECT oid FROM pg_catalog.pg_tablespace WHERE spcname='testspace' .... ) Do you think it's a good idea to list the files in the directory of the tablespace to ensure temporary table files are created where they should? Do you think there is a smart way to ensure the same with sort files? Any feedback welcome! A Dimecres 25 Octubre 2006 00:45, Albert Cervera Areny va escriure: > Hi, > > I'm trying to introduce myself into postgresql development and I'm working > on the "tablespace for temporary objects and sort files" TODO item. The > attached patch shows what I've already done. The GUC is currently > called "temp_tablespaces". > > The tablespace changes correctly for me when creating temporary tables. > I've got some questions though: > > How can I test that the tablespace is correctly used for sort files? Is > there an easy way? Or should I reduce work_mem to a minimum, populate the > database with data and try an "ORDER BY"? > > The GetTempTablespace function correctly returns a different tablespace > each time is called, but I store the position of the last tablespace used > with an integer and iterate through the list of tablespaces each time. I > tried to keep the iterator from call to call but I got a segfault, I > imagine due to the memory context. Should I try to keep the iterator? How > can I do it? > > Hope the diff and idents are ok. Please let me know if there's something > wrong with them. > > Thanks!
Attachment
On 12/27/06, Albert Cervera Areny <albertca@hotpop.com> wrote: > Hi, > here's a new version of the patch against HEAD with both, table and sort > files working correctly for me. Regression tests work too. ok, i will test it a little ... what about temp tables created by SELECT INTO TEMP? look at src/backend/executor/execMain.c:OpenIntoRel() what about other temporary objects? > I'd like to ask again the question I made on the first post as no answer was > given at that time: > > "The GetTempTablespace function correctly returns a different tablespace > each time is called, but I store the position of the last tablespace used > with an integer and iterate through the list of tablespaces each time. I > tried to keep the iterator from call to call but I got a segfault, I > imagine due to the memory context. Should I try to keep the iterator? How > can I do it?" > i didn't read this last time, i will take a look at it now... when you post for the first time hackers where busy releasing 8.2.0, maybe they will pay more atention now :) -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
On 12/27/06, Albert Cervera Areny <albertca@hotpop.com> wrote: > Hi, > here's a new version of the patch against HEAD with both, table and sort > files working correctly for me. Regression tests work too. > I'd like to ask again the question I made on the first post as no answer was > given at that time: > > "The GetTempTablespace function correctly returns a different tablespace > each time is called, but I store the position of the last tablespace used > with an integer and iterate through the list of tablespaces each time. I > tried to keep the iterator from call to call but I got a segfault, I > imagine due to the memory context. Should I try to keep the iterator? How > can I do it?" > > Now I'm working on some regression tests that could be added to > tablespace.source using something like: > > SET temp_tablespaces='testspace'; > > CREATE TEMP TABLE temp_foo(a VARCHAR); > > SELECT COUNT(pg_ls_dir('pg_tblspc/' || (SELECT oid FROM > pg_catalog.pg_tablespace WHERE spcname='testspace' .... ) > seems is working fine... i actually looked for the files in the tablespace directory... have you looked my past comments? -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Here's a new version that takes into account the SELECT INTO TEMP case. Thanks Jaime! What other temporary objects do you think should be considered? Any other comments on the patch? A Dijous 04 Gener 2007 05:33, Jaime Casanova va escriure: > On 12/27/06, Albert Cervera Areny <albertca@hotpop.com> wrote: > > Hi, > > here's a new version of the patch against HEAD with both, table > > and sort files working correctly for me. Regression tests work too. > > I'd like to ask again the question I made on the first post as no > > answer was given at that time: > > > > "The GetTempTablespace function correctly returns a different tablespace > > each time is called, but I store the position of the last tablespace used > > with an integer and iterate through the list of tablespaces each time. I > > tried to keep the iterator from call to call but I got a segfault, I > > imagine due to the memory context. Should I try to keep the iterator? How > > can I do it?" > > > > Now I'm working on some regression tests that could be added to > > tablespace.source using something like: > > > > SET temp_tablespaces='testspace'; > > > > CREATE TEMP TABLE temp_foo(a VARCHAR); > > > > SELECT COUNT(pg_ls_dir('pg_tblspc/' || (SELECT oid FROM > > pg_catalog.pg_tablespace WHERE spcname='testspace' .... ) > > seems is working fine... i actually looked for the files in the > tablespace directory... > have you looked my past comments?
Attachment
On 1/5/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > Here's a new version that takes into account the SELECT INTO TEMP case. Thanks > Jaime! > ok. seems good to me... > What other temporary objects do you think should be considered? > sorry for the noise, seems i'm thinking on other dbms... i really thought sequences use tablespaces but it seems i was wrong... maybe once this patch is applied you can think on make indexes and [temp] sequences on temp tables use the same temp_tablespace that table is using... > Any other comments on the patch? > yes. there are 2 things missing: the first one is put temp_tablespaces in postgresql.conf and/or making it a per database parameter (i don't think it should be a per user parameter) the second are docs... ;) -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
On 1/8/07, Jaime Casanova <systemguards@gmail.com> wrote: > maybe once this patch is applied you can think on make indexes and > [temp] sequences on temp tables use the same temp_tablespace that > table is using... > actually, the index part is easy... do you want to do it? -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
I don't have much time lately so if you're willing to work on it, please do. A Dimarts 09 Gener 2007 02:51, Jaime Casanova va escriure: > On 1/8/07, Jaime Casanova <systemguards@gmail.com> wrote: > > maybe once this patch is applied you can think on make indexes and > > [temp] sequences on temp tables use the same temp_tablespace that > > table is using... > > actually, the index part is easy... do you want to do it?
On 1/9/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > I don't have much time lately so if you're willing to work on it, please do. > after doing the index part i revisited the patch again and saw that there is something fundamentally wrong here (sorry for no noticing that before :( ) your are using local backend variables for the iterator so two different backends will use to different variables... because of that if you have 3 temp tablespaces in your temp_tablespaces guc and start 100 backend and every one of them create just one temp table those 100 temp tables will be in the first temp tablespace... :( i will try to fix that as well... unless you want to do it, just tell me... -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Please, go on with that, I hadn't seen that problem. Indeed, I read Andrew answer to your question and I think it's a nice solution. A Dimecres 10 Gener 2007 05:33, Jaime Casanova va escriure: > On 1/9/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > > I don't have much time lately so if you're willing to work on it, please > > do. > > after doing the index part i revisited the patch again and saw that > there is something fundamentally wrong here (sorry for no noticing > that before :( ) > > your are using local backend variables for the iterator so two > different backends will use to different variables... because of that > if you have 3 temp tablespaces in your temp_tablespaces guc and start > 100 backend and every one of them create just one temp table those 100 > temp tables will be in the first temp tablespace... :( > > i will try to fix that as well... unless you want to do it, just tell me...
On 1/11/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > Please, go on with that, I hadn't seen that problem. Indeed, I read Andrew > answer to your question and I think it's a nice solution. > yes... i'm always trying to kill flies with tanks... ;) i will use Andrew's suggestion... -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
On Thu, 2007-01-11 at 21:05 -0500, Jaime Casanova wrote: > On 1/11/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > > Please, go on with that, I hadn't seen that problem. Indeed, I read Andrew > > answer to your question and I think it's a nice solution. > > > > yes... i'm always trying to kill flies with tanks... ;) Isn't that a little expensive on gas? > i will use Andrew's suggestion... > -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutions since 1997 http://www.commandprompt.com/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
On 1/11/07, Joshua D. Drake <jd@commandprompt.com> wrote: > On Thu, 2007-01-11 at 21:05 -0500, Jaime Casanova wrote: > > On 1/11/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > > > Please, go on with that, I hadn't seen that problem. Indeed, I read Andrew > > > answer to your question and I think it's a nice solution. > > > > > > > yes... i'm always trying to kill flies with tanks... ;) > > Isn't that a little expensive on gas? > maybe... but... what a beatiful explosion we can get... ;) seriously, attached's a new version of the patch... the patch use temp tablespaces for: - temp tables - temp files (generated by sorts and such) - indexes on temp tables the temp_tablespaces GUC still cannot be set from postgresql.conf, i will keep working on that but i have to understand the code... any comments on this patch? although it's not ready for apply it yet, i think Albert made a good work on it... -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Sorry, patch attached this time... On 1/12/07, Jaime Casanova <systemguards@gmail.com> wrote: > On 1/11/07, Joshua D. Drake <jd@commandprompt.com> wrote: > > On Thu, 2007-01-11 at 21:05 -0500, Jaime Casanova wrote: > > > On 1/11/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > > > > Please, go on with that, I hadn't seen that problem. Indeed, I read Andrew > > > > answer to your question and I think it's a nice solution. > > > > > > > > > > yes... i'm always trying to kill flies with tanks... ;) > > > > Isn't that a little expensive on gas? > > > > maybe... but... what a beatiful explosion we can get... ;) > > seriously, attached's a new version of the patch... > the patch use temp tablespaces for: > - temp tables > - temp files (generated by sorts and such) > - indexes on temp tables > > the temp_tablespaces GUC still cannot be set from postgresql.conf, i > will keep working on that but i have to understand the code... > > any comments on this patch? although it's not ready for apply it yet, > i think Albert made a good work on it... > > -- > regards, > Jaime Casanova > > "Programming today is a race between software engineers striving to > build bigger and better idiot-proof programs and the universe trying > to produce bigger and better idiots. > So far, the universe is winning." > Richard Cook > -- Atentamente, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Attachment
It was already possible to set the guc on postgresql.conf when I posted the patch... A Divendres 12 Gener 2007 07:28, Jaime Casanova va escriure: > On 1/11/07, Joshua D. Drake <jd@commandprompt.com> wrote: > > On Thu, 2007-01-11 at 21:05 -0500, Jaime Casanova wrote: > > > On 1/11/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > > > > Please, go on with that, I hadn't seen that problem. Indeed, I read > > > > Andrew answer to your question and I think it's a nice solution. > > > > > > yes... i'm always trying to kill flies with tanks... ;) > > > > Isn't that a little expensive on gas? > > maybe... but... what a beatiful explosion we can get... ;) > > seriously, attached's a new version of the patch... > the patch use temp tablespaces for: > - temp tables > - temp files (generated by sorts and such) > - indexes on temp tables > > the temp_tablespaces GUC still cannot be set from postgresql.conf, i > will keep working on that but i have to understand the code... > > any comments on this patch? although it's not ready for apply it yet, > i think Albert made a good work on it...
On 1/13/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > It was already possible to set the guc on postgresql.conf when I posted the > patch... > ok... fixed... the problem was that this code only let num_temp_tablespaces be greater than zero when we are in an interactive command (eg. a SET command) but setting the guc from postgresql.conf at startup time is not interactive so num_temp_tablespaces is zero and when i try to get the first temp tablespace to use (MyProcPid % num_temp_tablespaces) causes a floatin exception (division by zero). + if (source >= PGC_S_INTERACTIVE && IsTransactionState()) + { + /* + * Verify that all the names are valid tablspace names + * We do not check for USAGE rights should we? + */ + foreach(l, namelist) + { + char *curname = (char *) lfirst(l); + + if (get_tablespace_oid(curname) == InvalidOid) + ereport((source == PGC_S_TEST) ? NOTICE : ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("tablespace \"%s\" does not exist", curname))); + + num_temp_tablespaces++; + } + } new patch added, with that piece of code refactored to let num_temp_tablespaces get a value greater than zero always that the guc is setted, i also add some docs. the patch passes all 104 regression tests and all my tests as well... i think the patch is ready to be applied to HEAD, any committer want to review it? -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Attachment
On Sunday 14 January 2007 23:16, Jaime Casanova wrote: > On 1/13/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > > It was already possible to set the guc on postgresql.conf when I posted > > the patch... > > ok... fixed... the problem was that this code only let > num_temp_tablespaces be greater than zero when we are in an > interactive command (eg. a SET command) but setting the guc from > postgresql.conf at startup time is not interactive so > num_temp_tablespaces is zero and when i try to get the first temp > tablespace to use (MyProcPid % num_temp_tablespaces) causes a floatin > exception (division by zero). <snip> > new patch added, with that piece of code refactored to let > num_temp_tablespaces get a value greater than zero always that the guc > is setted, i also add some docs. > > the patch passes all 104 regression tests and all my tests as well... > > i think the patch is ready to be applied to HEAD, any committer want > to review it? Why is this PGC_SUSET? ISTM it should be PGC_USERSET. -- Robert Treat Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
A Dimecres 17 Gener 2007 22:38, Robert Treat va escriure: > Why is this PGC_SUSET? ISTM it should be PGC_USERSET. Yes, I think your're right and it should be PGC_USERSET.
Patch withdrawn by author; corrected version expected. --------------------------------------------------------------------------- Jaime Casanova wrote: > On 1/13/07, Albert Cervera Areny <albertca@hotpop.com> wrote: > > It was already possible to set the guc on postgresql.conf when I posted the > > patch... > > > > ok... fixed... the problem was that this code only let > num_temp_tablespaces be greater than zero when we are in an > interactive command (eg. a SET command) but setting the guc from > postgresql.conf at startup time is not interactive so > num_temp_tablespaces is zero and when i try to get the first temp > tablespace to use (MyProcPid % num_temp_tablespaces) causes a floatin > exception (division by zero). > > + if (source >= PGC_S_INTERACTIVE && IsTransactionState()) > + { > + /* > + * Verify that all the names are valid tablspace names > + * We do not check for USAGE rights should we? > + */ > + foreach(l, namelist) > + { > + char *curname = (char *) lfirst(l); > + > + if (get_tablespace_oid(curname) == InvalidOid) > + ereport((source == PGC_S_TEST) ? NOTICE : ERROR, > + (errcode(ERRCODE_UNDEFINED_OBJECT), > + errmsg("tablespace \"%s\" does not exist", curname))); > + > + num_temp_tablespaces++; > + } > + } > > > new patch added, with that piece of code refactored to let > num_temp_tablespaces get a value greater than zero always that the guc > is setted, i also add some docs. > > the patch passes all 104 regression tests and all my tests as well... > > i think the patch is ready to be applied to HEAD, any committer want > to review it? > > -- > regards, > Jaime Casanova > > "Programming today is a race between software engineers striving to > build bigger and better idiot-proof programs and the universe trying > to produce bigger and better idiots. > So far, the universe is winning." > Richard Cook [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On 1/20/07, Bruce Momjian <bruce@momjian.us> wrote: > > Patch withdrawn by author; corrected version expected. > new version of Albert's patch with PGC_USERSET instead of PGC_SUSET -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
Attachment
Patch applied. Thanks. --------------------------------------------------------------------------- Jaime Casanova wrote: > On 1/20/07, Bruce Momjian <bruce@momjian.us> wrote: > > > > Patch withdrawn by author; corrected version expected. > > > > new version of Albert's patch with PGC_USERSET instead of PGC_SUSET > > -- > regards, > Jaime Casanova > > "Programming today is a race between software engineers striving to > build bigger and better idiot-proof programs and the universe trying > to produce bigger and better idiots. > So far, the universe is winning." > Richard Cook [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +