Обсуждение: CREATE TABLESPACE SET
I was recently annoyed that I had to do CREATE TABLESPACE x LOCATION y; ALTER TABLESPACE x SET (random_page_cost = z); The attached patch is a quick n' dirty extension to allow the SET directly on the CREATE statement. CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); If people think this is a good idea, I'll clean it up, add documentation, and submit it to the next commitfest. -- Vik
Вложения
On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote: > I was recently annoyed that I had to do > > CREATE TABLESPACE x LOCATION y; > ALTER TABLESPACE x SET (random_page_cost = z); > > The attached patch is a quick n' dirty extension to allow the SET > directly on the CREATE statement. > > CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); That should probably be WITH instead of SET for consistency with other similar DDL. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On 12/26/2013 06:10 PM, David Fetter wrote: > On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote: >> I was recently annoyed that I had to do >> >> CREATE TABLESPACE x LOCATION y; >> ALTER TABLESPACE x SET (random_page_cost = z); >> >> The attached patch is a quick n' dirty extension to allow the SET >> directly on the CREATE statement. >> >> CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); > That should probably be WITH instead of SET for consistency with other > similar DDL. It would make a simpler patch, too, but I wanted to be consistent with ALTER TABLESPACE. I have no firm opinion on the subject, so I'll switch it to WITH if no one else says anything. -- Vik
On 12/26/2013 06:10 PM, David Fetter wrote: > On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote: >> I was recently annoyed that I had to do >> >> CREATE TABLESPACE x LOCATION y; >> ALTER TABLESPACE x SET (random_page_cost = z); >> >> The attached patch is a quick n' dirty extension to allow the SET >> directly on the CREATE statement. >> >> CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); > That should probably be WITH instead of SET for consistency with other > similar DDL. Here is version 2 of the patch, which uses WITH instead of SET, and also adds to the documentation. -- Vik
Вложения
On Wed, Jan 15, 2014 at 10:27 AM, Vik Fearing <vik.fearing@dalibo.com> wrote: > On 12/26/2013 06:10 PM, David Fetter wrote: >> On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote: >>> I was recently annoyed that I had to do >>> >>> CREATE TABLESPACE x LOCATION y; >>> ALTER TABLESPACE x SET (random_page_cost = z); >>> >>> The attached patch is a quick n' dirty extension to allow the SET >>> directly on the CREATE statement. >>> >>> CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); >> That should probably be WITH instead of SET for consistency with other >> similar DDL. > > Here is version 2 of the patch, which uses WITH instead of SET, and also > adds to the documentation. I just had a quick look at this patch, no testing at all. I am seeing that regression tests are still missing here, they should be added in src/test/regress/input/tablespace.source. Then, for the use of either WITH or SET... For example CREATE FUNCTION uses SET for a configuration parameter, and ALTER FUNCTION is consistent with that. So SET makes more sense to be consistent with CREATE TABLESPACE? This patch should not be modified once again as long as there are no more opinions though... Regards, -- Michael
On 1/14/14, 8:07 PM, Michael Paquier wrote: > On Wed, Jan 15, 2014 at 10:27 AM, Vik Fearing <vik.fearing@dalibo.com> wrote: >> On 12/26/2013 06:10 PM, David Fetter wrote: >>> On Tue, Dec 24, 2013 at 07:25:01PM +0100, Vik Fearing wrote: >>>> I was recently annoyed that I had to do >>>> >>>> CREATE TABLESPACE x LOCATION y; >>>> ALTER TABLESPACE x SET (random_page_cost = z); >>>> >>>> The attached patch is a quick n' dirty extension to allow the SET >>>> directly on the CREATE statement. >>>> >>>> CREATE TABLESPACE x LOCATION y SET (random_page_cost = z); >>> That should probably be WITH instead of SET for consistency with other >>> similar DDL. >> >> Here is version 2 of the patch, which uses WITH instead of SET, and also >> adds to the documentation. > I just had a quick look at this patch, no testing at all. I am seeing > that regression tests are still missing here, they should be added in > src/test/regress/input/tablespace.source. Then, for the use of either > WITH or SET... For example CREATE FUNCTION uses SET for a > configuration parameter, and ALTER FUNCTION is consistent with that. > So SET makes more sense to be consistent with CREATE TABLESPACE? This > patch should not be modified once again as long as there are no more > opinions though... You know, this doesn't do much to encourage people to submit patches since it was just suggested that we use WITH insteadof SET. :( Anyone have an easy way to see which is more prevalent? I'd be stuck with \h or trying to grep SGML... I'm hoping someoneelse has an easier way... -- Jim C. Nasby, Data Architect jim@nasby.net 512.569.9461 (cell) http://jim.nasby.net
On Wed, Jan 15, 2014 at 2:02 PM, Jim Nasby <jim@nasby.net> wrote:
> You know, this doesn't do much to encourage people to submit patches since
> it was just suggested that we use WITH instead of SET. :(
Sorry, you are right.
> Anyone have an easy way to see which is more prevalent? I'd be stuck with \h
> or trying to grep SGML... I'm hoping someone else has an easier way...
I am seeing that the "SET ()" pattern is used in those files:
$ cd postgres/doc/src/sgml/ref
$ git grep -e " SET (" --and -e "replaceable" --name-only
alter_foreign_table.sgml
alter_index.sgml
alter_materialized_view.sgml
alter_table.sgml
alter_tablespace.sgml
alter_view.sgml
While the WITH () pattern is used in those files:
$ git grep -e " WITH (" --and -e "replaceable" --name-only
create_function.sgml
create_index.sgml
create_materialized_view.sgml
create_table.sgml
create_table_as.sgml
create_view.sgml
So yes WITH makes sense for a CREATE command.
Regards,
--
Michael
On 01/15/2014 03:07 AM, Michael Paquier wrote: > I just had a quick look at this patch, no testing at all. Thank you for looking at it. > I am seeing that regression tests are still missing here, they should be added in > src/test/regress/input/tablespace.source. New patch attached, with regression tests. > Then, for the use of either > WITH or SET... For example CREATE FUNCTION uses SET for a > configuration parameter, and ALTER FUNCTION is consistent with that. > So SET makes more sense to be consistent with CREATE TABLESPACE? This > patch should not be modified once again as long as there are no more > opinions though... Based on your analysis of current behavior elsewhere, and the messier patch with SET, I have switched my vote from ambivalent to preferring WITH. -- Vik
Вложения
On Thu, Jan 16, 2014 at 9:03 AM, Vik Fearing <vik.fearing@dalibo.com> wrote: > New patch attached, with regression tests. Thanks for the new version, I have spent some time looking at it: - Patch compiles without warnings. - Done some manual testing with CREATE/ALTER TABLESPACE WITH and checked pg_tablespace, it worked fine. - However, regression tests are failing because src/test/regress/output/tablespace.source has an incorrect output, it is necessary to replace that: /home/vik/postgresql/9.4/git/src/test/regress/testtablespace by that: @testtablespace@ This is something I have actually fixed that in the attached patch. So, except for the regression output problem, I think that the code is clean so marking it as "Ready for committer" on the commit fest app. Thanks, -- Michael
Вложения
On 01/18/2014 03:21 PM, Michael Paquier wrote: > So, except for the regression output problem, I think that the code is > clean so marking it as "Ready for committer" on the commit fest app. Thank you for the review. Sorry about the regression thing. I tried to randomize it a bit so that I would catch it but it ended up breaking all the other tablespace tests, so I decided it's not worth it. -- Vik