Thread: CREATE SCHEMA IF NOT EXISTS
Hackers, Is there any reason not to add $subject? Would it be difficult? Would save me having to write a DO statement every time Ineeded it (which in migration scripts is fairly often). Thanks, David
2012/8/15 David E. Wheeler <david@justatheory.com>
Hackers,
Is there any reason not to add $subject? Would it be difficult?
Looking to the source code I think this feature isn't hard to implement... I'm writing a little path to do that and I'll send soon...
Would save me having to write a DO statement every time I needed it (which in migration scripts is fairly often).
Regards,
--
Fabrízio de Royes Mello
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
On Aug 15, 2012, at 11:31 AM, Fabrízio de Royes Mello wrote: >> Is there any reason not to add $subject? Would it be difficult? > > Looking to the source code I think this feature isn't hard to implement... I'm writing a little path to do that and I'llsend soon... Cool, thanks! David
2012/8/15 David E. Wheeler <david@justatheory.com>
On Aug 15, 2012, at 11:31 AM, Fabrízio de Royes Mello wrote:Cool, thanks!
>> Is there any reason not to add $subject? Would it be difficult?
>
> Looking to the source code I think this feature isn't hard to implement... I'm writing a little path to do that and I'll send soon...
CREATE SCHEMA [ IF NOT EXISTS ] schema_name [ AUTHORIZATION user_name ] [ schema_element [ ... ] ]
CREATE SCHEMA [ IF NOT EXISTS ] AUTHORIZATION user_name [ schema_element [ ... ] ]
Now, PostgreSQL don't trow an error if we use "IF NOT EXISTS" in "CREATE SCHEMA" statement.
So, I don't know the next steps...
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Attachment
On 08/16/2012 01:36 PM, Fabrízio de Royes Mello wrote: > > > 2012/8/15 David E. Wheeler <david@justatheory.com > <mailto:david@justatheory.com>> > > On Aug 15, 2012, at 11:31 AM, Fabrízio de Royes Mello wrote: > > >> Is there any reason not to add $subject? Would it be difficult? > > > > Looking to the source code I think this feature isn't hard to > implement... I'm writing a little path to do that and I'll send > soon... > > Cool, thanks! > > > The attached patch implement this feature: > > CREATE SCHEMA [ IF NOT EXISTS ] schema_name [ AUTHORIZATION user_name > ] [ schema_element [ ... ] ] > CREATE SCHEMA [ IF NOT EXISTS ] AUTHORIZATION user_name [ > schema_element [ ... ] ] > > Now, PostgreSQL don't trow an error if we use "IF NOT EXISTS" in > "CREATE SCHEMA" statement. > > So, I don't know the next steps... Please see <http://wiki.postgresql.org/wiki/Developer_FAQ#I_have_developed_a_patch.2C_what_next.3F> cheers andrew
On Aug 16, 2012, at 10:36 AM, Fabrízio de Royes Mello wrote: > The attached patch implement this feature: > > CREATE SCHEMA [ IF NOT EXISTS ] schema_name [ AUTHORIZATION user_name ] [ schema_element [ ... ] ] > CREATE SCHEMA [ IF NOT EXISTS ] AUTHORIZATION user_name [ schema_element [ ... ] ] > > Now, PostgreSQL don't trow an error if we use "IF NOT EXISTS" in "CREATE SCHEMA" statement. > > So, I don't know the next steps... Awesome, thanks! Please add it to the next CommitFest: https://commitfest.postgresql.org/action/commitfest_view?id=15 Best, David
2012/8/16 David E. Wheeler <david@justatheory.com>
On Aug 16, 2012, at 10:36 AM, Fabrízio de Royes Mello wrote:Awesome, thanks! Please add it to the next CommitFest:
> The attached patch implement this feature:
>
> CREATE SCHEMA [ IF NOT EXISTS ] schema_name [ AUTHORIZATION user_name ] [ schema_element [ ... ] ]
> CREATE SCHEMA [ IF NOT EXISTS ] AUTHORIZATION user_name [ schema_element [ ... ] ]
>
> Now, PostgreSQL don't trow an error if we use "IF NOT EXISTS" in "CREATE SCHEMA" statement.
>
> So, I don't know the next steps...
https://commitfest.postgresql.org/action/commitfest_view?id=15
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
2012/8/16 Fabrízio de Royes Mello <fabriziomello@gmail.com>: > The attached patch implement this feature: > > CREATE SCHEMA [ IF NOT EXISTS ] schema_name [ AUTHORIZATION user_name ] [ > schema_element [ ... ] ] > CREATE SCHEMA [ IF NOT EXISTS ] AUTHORIZATION user_name [ schema_element [ > ... ] ] > > Now, PostgreSQL don't trow an error if we use "IF NOT EXISTS" in "CREATE > SCHEMA" statement. I started testing this, but I didn't see regression tests for it. Could you write them?. Best. -- Dickson S. Guedes mail/xmpp: guedes@guedesoft.net - skype: guediz http://guedesoft.net - http://www.postgresql.org.br
2012/8/16 Dickson S. Guedes <listas@guedesoft.net>
2012/8/16 Fabrízio de Royes Mello <fabriziomello@gmail.com>:> The attached patch implement this feature:I started testing this, but I didn't see regression tests for it.
>
> CREATE SCHEMA [ IF NOT EXISTS ] schema_name [ AUTHORIZATION user_name ] [
> schema_element [ ... ] ]
> CREATE SCHEMA [ IF NOT EXISTS ] AUTHORIZATION user_name [ schema_element [
> ... ] ]
>
> Now, PostgreSQL don't trow an error if we use "IF NOT EXISTS" in "CREATE
> SCHEMA" statement.
Could you write them?.
The attached patch contains regression tests for it.
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Attachment
2012/8/17 Fabrízio de Royes Mello <fabriziomello@gmail.com>
I started testing this, but I didn't see regression tests for it.Could you write them?.The attached patch contains regression tests for it.
Regards,
--
Fabrízio de Royes Mello
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Attachment
Excerpts from Fabrízio de Royes Mello's message of vie ago 17 09:16:30 -0400 2012: > The attached patch contains regression tests for it. I think you're missing support in copyfuncs.c and equalfuncs.c for the new field in the node. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
2012/8/17 Alvaro Herrera <alvherre@2ndquadrant.com>
Excerpts from Fabrízio de Royes Mello's message of vie ago 17 09:16:30 -0400 2012:I think you're missing support in copyfuncs.c and equalfuncs.c for the
> The attached patch contains regression tests for it.
new field in the node.
The attached patch add support for the new field in the node in "copyfuncs.c" and "equalfuncs.c".
--
Fabrízio de Royes Mello
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Attachment
2012/8/17 Fabrízio de Royes Mello <fabriziomello@gmail.com>: > > 2012/8/17 Alvaro Herrera <alvherre@2ndquadrant.com> >> >> Excerpts from Fabrízio de Royes Mello's message of vie ago 17 09:16:30 >> -0400 2012: >> >> > The attached patch contains regression tests for it. >> >> I think you're missing support in copyfuncs.c and equalfuncs.c for the >> new field in the node. > > You're completely right, thanks... > > The attached patch add support for the new field in the node in > "copyfuncs.c" and "equalfuncs.c". Maybe I'm missing something but shouldn't it being handled in extension.c too? []s -- Dickson S. Guedes mail/xmpp: guedes@guedesoft.net - skype: guediz http://guedesoft.net - http://www.postgresql.org.br
2012/8/17 Alvaro Herrera <alvherre@2ndquadrant.com>
Excerpts from Dickson S. Guedes's message of vie ago 17 10:37:25 -0400 2012:> 2012/8/17 Fabrízio de Royes Mello <fabriziomello@gmail.com>:Please be more explicit? I don't know what you mean.
> >
> > 2012/8/17 Alvaro Herrera <alvherre@2ndquadrant.com>
> >>
> >> Excerpts from Fabrízio de Royes Mello's message of vie ago 17 09:16:30
> >> -0400 2012:
> >>
> >> > The attached patch contains regression tests for it.
> >>
> >> I think you're missing support in copyfuncs.c and equalfuncs.c for the
> >> new field in the node.
> >
> > You're completely right, thanks...
> >
> > The attached patch add support for the new field in the node in
> > "copyfuncs.c" and "equalfuncs.c".
>
> Maybe I'm missing something but shouldn't it being handled in extension.c too?
Returning conversation to the list.
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 8512cdb..e359a9c 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1374,6 +1374,7 @@ CreateExtension(CreateExtensionStmt *stmt)
csstmt->schemaname = schemaName;
csstmt->authid = NULL; /* will be created by current user */
csstmt->schemaElts = NIL;
+ csstmt->if_not_exists = false;
CreateSchemaCommand(csstmt, NULL);
/*
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Attachment
Hello, I reviewed this v5 of patch: - https://commitfest.postgresql.org/action/patch_view?id=907 The patch is small and implements a new syntax to CREATE SCHEMA that allow the creation of a schema be skipped when IF NOT EXISTS is used. It was applied to 483c2c1071c45e275782d33d646c3018f02f9f94 with two hunks offset, was compiled without errors or new warnings and pass all tests, even the tests that covers the expected results for it self. The docs was updated with the information about the expected behavior. I tested against ambiguity, i. e. creating a schema with name 'if' and got the expected results when try to create it if not exists. Two questions: - Should this patch implements others INEs like ADD COLUMN IF NOT EXISTS? - Should pg_dump or pg_restore support some kind of flag to use a CREATE SCHEMA IF NOT EXISTS ... instead CREATE SCHEMA ...? Regards, -- Dickson S. Guedes mail/xmpp: guedes@guedesoft.net - skype/twitter: guediz ~ github.com/guedes http://guedesoft.net ~ http://www.postgresql.org.br
Attachment
2012/8/27 Dickson S. Guedes <listas@guedesoft.net>
[...]
Two questions:
- Should this patch implements others INEs like ADD COLUMN IF NOT EXISTS?
If this feature is important I believe we must implement it.
Exists several "CREATE" statements without "IF NOT EXISTS" option too, so we can discuss more about it and I can implement it in this patch or in another.
- Should pg_dump or pg_restore support some kind of flag to use a
CREATE SCHEMA IF NOT EXISTS ... instead CREATE SCHEMA ...?
I can't imagine a use case for this feature... can you explain more about your idea?
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
On Aug 28, 2012, at 8:19 AM, Fabrízio de Royes Mello wrote: >> - Should this patch implements others INEs like ADD COLUMN IF NOT EXISTS? > > If this feature is important I believe we must implement it. > > Exists several "CREATE" statements without "IF NOT EXISTS" option too, so we can discuss more about it and I can implementit in this patch or in another. I don't think any of them have to be a part of *this* patch. There can be patches for each of them. Probably simpler thatway. But then, I am not like to code them, so WTF do I know? :-) David
2012/8/28 David E. Wheeler <david@justatheory.com>: > On Aug 28, 2012, at 8:19 AM, Fabrízio de Royes Mello wrote: > >>> - Should this patch implements others INEs like ADD COLUMN IF NOT EXISTS? >> >> If this feature is important I believe we must implement it. >> >> Exists several "CREATE" statements without "IF NOT EXISTS" option too, so we can discuss more about it and I can implementit in this patch or in another. > > I don't think any of them have to be a part of *this* patch. There can be patches for each of them. Probably simpler thatway. Yes, agreed. Other implementations should be in their own distinct patches. BTW, it could be interesting search the archive for past discussions about CINE. I changed the status of this patch to "Ready for Committer". Regards, -- Dickson S. Guedes mail/xmpp: guedes@guedesoft.net - skype/twitter: guediz ~ github.com/guedes http://guedesoft.net ~ http://www.postgresql.org.br
"Dickson S. Guedes" <listas@guedesoft.net> writes: > I reviewed this v5 of patch: > - https://commitfest.postgresql.org/action/patch_view?id=907 > The patch is small and implements a new syntax to CREATE SCHEMA > that allow the creation of a schema be skipped when IF NOT EXISTS is > used. I don't believe this has been thought through nearly carefully enough. If CREATE SCHEMA created a schema and nothing more, then the proposed implementation would probably be fine. But per spec, CREATE SCHEMA can specify not only creating the schema but a whole bunch of objects within the schema. As coded, if the schema exists then creation of the specified sub-objects is just skipped, regardless of whether they exist or not. I doubt that this is really sane behavior. Would the principle of least astonishment dictate that the IF NOT EXISTS option apply implicitly to each sub-object as well? (If so, we'd have to extend everything that can appear in OptSchemaEltList; most of those commands don't have IF NOT EXISTS options today.) This is somewhat connected to our previous arguments about whether CINE ought to try to make any guarantees about whether the existing object is at all like the object that would have been created if the statement executed successfully. I realize that the existing statements that have an INE option have entirely blown off this concern, but I still think that it's a question that ought to be considered, particularly for objects that have separately creatable sub-structure. A possible compromise is to allow the IF NOT EXISTS option only without a schema-element list, which I suspect is the only use-case David had in mind to start with anyway. The existing patch added the check in a pretty randomly chosen spot too, with one bad consequence being that if the schema already exists then it will fall out with the wrong user ID in effect, creating a security bug. But I'm not entirely sure where to put the check instead. Should we put it before or after the permissions checks --- that is, should IF NOT EXISTS require that you would have had permission to create the schema? Or, if the schema does exist, should we just call it good anyway? I'm too lazy to look at how other INE options resolved this question, but it seems like we ought to be consistent. Also, the AUTHORIZATION clause of CREATE SCHEMA creates an aspect of all this that doesn't exist for any other kind of CREATE command, namely that the object might have been requested to be created under some other user id. For instance, supposing that we were to go forward with trying to create sub-objects, but the ownership of the existing schema is different from what's implied or specified by CREATE SCHEMA, should the sub-objects be (attempted to be) created as owned by that user instead? Perhaps not, but I'm not at all sure. regards, tom lane
On Sep 21, 2012, at 10:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I don't believe this has been thought through nearly carefully enough. > If CREATE SCHEMA created a schema and nothing more, then the proposed > implementation would probably be fine. But per spec, CREATE SCHEMA > can specify not only creating the schema but a whole bunch of objects > within the schema. As coded, if the schema exists then creation of > the specified sub-objects is just skipped, regardless of whether they > exist or not. I doubt that this is really sane behavior. Would the > principle of least astonishment dictate that the IF NOT EXISTS option > apply implicitly to each sub-object as well? (If so, we'd have to > extend everything that can appear in OptSchemaEltList; most of those > commands don't have IF NOT EXISTS options today.) I had no idea about that functionality. Seems very strange. > A possible compromise is to allow the IF NOT EXISTS option only without > a schema-element list, which I suspect is the only use-case David had in > mind to start with anyway. Yes, true. > The existing patch added the check in a pretty randomly chosen spot too, > with one bad consequence being that if the schema already exists then it > will fall out with the wrong user ID in effect, creating a security bug. > But I'm not entirely sure where to put the check instead. Should we put > it before or after the permissions checks --- that is, should IF NOT > EXISTS require that you would have had permission to create the schema? > Or, if the schema does exist, should we just call it good anyway? I'm > too lazy to look at how other INE options resolved this question, but it > seems like we ought to be consistent. Agreed. But if it already exists, where does it currently die? ISTM that would be the point to check, if possible. > Also, the AUTHORIZATION clause of CREATE SCHEMA creates an aspect of all > this that doesn't exist for any other kind of CREATE command, namely > that the object might have been requested to be created under some other > user id. For instance, supposing that we were to go forward with trying > to create sub-objects, but the ownership of the existing schema is > different from what's implied or specified by CREATE SCHEMA, should the > sub-objects be (attempted to be) created as owned by that user instead? > Perhaps not, but I'm not at all sure. I tend to think that if the schema exists, there should be no attempt to create the sub-objects. Seems the least astonishingto me. Best, David
On Sat, Sep 22, 2012 at 3:06 AM, David E. Wheeler <david@justatheory.com> wrote:
-- On Sep 21, 2012, at 10:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:I had no idea about that functionality. Seems very strange.
> I don't believe this has been thought through nearly carefully enough.
> If CREATE SCHEMA created a schema and nothing more, then the proposed
> implementation would probably be fine. But per spec, CREATE SCHEMA
> can specify not only creating the schema but a whole bunch of objects
> within the schema. As coded, if the schema exists then creation of
> the specified sub-objects is just skipped, regardless of whether they
> exist or not. I doubt that this is really sane behavior. Would the
> principle of least astonishment dictate that the IF NOT EXISTS option
> apply implicitly to each sub-object as well? (If so, we'd have to
> extend everything that can appear in OptSchemaEltList; most of those
> commands don't have IF NOT EXISTS options today.)
Isn't it this SQL?
CREATE SCHEMA foo CREATE TABLE aa (a int) CREATE TABLE bb (b int);
CREATE SCHEMA foo CREATE TABLE aa (a int) CREATE TABLE bb (b int);
Michael Paquier
http://michael.otacoo.com
2012/9/21 David E. Wheeler <david@justatheory.com>
On Sep 21, 2012, at 10:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:I had no idea about that functionality. Seems very strange.
> I don't believe this has been thought through nearly carefully enough.
> If CREATE SCHEMA created a schema and nothing more, then the proposed
> implementation would probably be fine. But per spec, CREATE SCHEMA
> can specify not only creating the schema but a whole bunch of objects
> within the schema. As coded, if the schema exists then creation of
> the specified sub-objects is just skipped, regardless of whether they
> exist or not. I doubt that this is really sane behavior. Would the
> principle of least astonishment dictate that the IF NOT EXISTS option
> apply implicitly to each sub-object as well? (If so, we'd have to
> extend everything that can appear in OptSchemaEltList; most of those
> commands don't have IF NOT EXISTS options today.)
I completely forgot this functionality. The example above is from our docs [1]:
CREATE SCHEMA hollywood
CREATE TABLE films (title text, release date, awards text[])
CREATE VIEW winners AS
SELECT title, release FROM films WHERE awards IS NOT NULL;
The "CREATE SCHEMA" statement accepts another "CREATE" commands (CREATE {TABLE | VIEW | INDEX | SEQUENCE | TRIGGER}), and the current patch do not consider this options.
> A possible compromise is to allow the IF NOT EXISTS option only withoutYes, true.
> a schema-element list, which I suspect is the only use-case David had in
> mind to start with anyway.
Ok.
> The existing patch added the check in a pretty randomly chosen spot too,Agreed. But if it already exists, where does it currently die? ISTM that would be the point to check, if possible.
> with one bad consequence being that if the schema already exists then it
> will fall out with the wrong user ID in effect, creating a security bug.
> But I'm not entirely sure where to put the check instead. Should we put
> it before or after the permissions checks --- that is, should IF NOT
> EXISTS require that you would have had permission to create the schema?
> Or, if the schema does exist, should we just call it good anyway? I'm
> too lazy to look at how other INE options resolved this question, but it
> seems like we ought to be consistent.
I change the patch (attached) to skip only the schema creation and execute others statements...
> Also, the AUTHORIZATION clause of CREATE SCHEMA creates an aspect of allI tend to think that if the schema exists, there should be no attempt to create the sub-objects. Seems the least astonishing to me.
> this that doesn't exist for any other kind of CREATE command, namely
> that the object might have been requested to be created under some other
> user id. For instance, supposing that we were to go forward with trying
> to create sub-objects, but the ownership of the existing schema is
> different from what's implied or specified by CREATE SCHEMA, should the
> sub-objects be (attempted to be) created as owned by that user instead?
> Perhaps not, but I'm not at all sure.
Why don't create sub-objects? I think the INE clause must affect only "CREATE SCHEMA" statement, the others must be executed normally. We can discuss more about it...
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Attachment
Dickson S. Guedes schrieb: > - https://commitfest.postgresql.org/action/patch_view?id=907 > > The patch is small and implements a new syntax to CREATE SCHEMA > that allow the creation of a schema be skipped when IF NOT EXISTS is > used. > > [...] > > - Should this patch implements others INEs like ADD COLUMN IF NOT EXISTS? If there's still a chance to improve the patch, I'd love to see the following INEs implemented. Several real-world database upgrade scripts would benefit from those: 1) CREATE [ TEMPORARY | TEMP ] SEQUENCE [ IF NOT EXISTS ] name ... 2) ALTER TABLE ... ADD [ COLUMN ] [ IF NOT EXISTS ] column_name ... 3) CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ IF NOT EXISTS ] [ name ] ON ... Regards, Volker -- Volker Grabsch ---<<(())>>---
The fundamental issue with this patch hasn't been answered sufficiently, I think. Consider the following sequence of commands: create schema if not exists foo create table first (a int); create schema if not exists foo create table second (a int); As far as I can see, with the patch as it currently stands, you would end up with only table "first" in the schema, which seems very surprising to me. I think this needs more thought, and in any case it needs more comprehensive regression test and documentation (i.e. at least the examples ought to explain what would happen in such cases). -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Excerpts from Volker Grabsch's message of sáb sep 29 06:32:13 -0300 2012: > Dickson S. Guedes schrieb: > > - https://commitfest.postgresql.org/action/patch_view?id=907 > > > > The patch is small and implements a new syntax to CREATE SCHEMA > > that allow the creation of a schema be skipped when IF NOT EXISTS is > > used. > > > > [...] > > > > - Should this patch implements others INEs like ADD COLUMN IF NOT EXISTS? > > If there's still a chance to improve the patch, I'd love to see > the following INEs implemented. Several real-world database > upgrade scripts would benefit from those: I don't see that this patch is responsible for such new commands. If you want them, feel free to submit separate patches for them (or have someone else do it for you). But see the thread starting at http://archives.postgresql.org/message-id/467881.79137.qm%40web27104.mail.ukl.yahoo.com -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Oct 2, 2012, at 12:08 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > create schema if not exists foo create table first (a int); > create schema if not exists foo create table second (a int); > > > As far as I can see, with the patch as it currently stands, you would > end up with only table "first" in the schema, which seems very > surprising to me. Yeah, I think the second should die. CINE should only work if there are no other objects created as part of the statement,IMHO. Best, David
Excerpts from David E. Wheeler's message of mar oct 02 16:16:37 -0300 2012: > On Oct 2, 2012, at 12:08 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > > > create schema if not exists foo create table first (a int); > > create schema if not exists foo create table second (a int); > > > > > > As far as I can see, with the patch as it currently stands, you would > > end up with only table "first" in the schema, which seems very > > surprising to me. > > Yeah, I think the second should die. CINE should only work if there are no other objects created as part of the statement,IMHO. Well, if that's the rationale then you end up with no schema foo at all (i.e. both die), which seems even more surprising (though I admit it has the advantage of being a simple rule to document.) How about call this for precedent: mkdir -p /tmp/foo/bar mkdir -p /tmp/foo/baz In this case you end up with directory "foo" and at least two subdirs in it, bar and baz. This works even if /tmp/foo existed previously and even if there was some other stuff in it. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Oct 2, 2012, at 12:30 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > How about call this for precedent: > > mkdir -p /tmp/foo/bar > mkdir -p /tmp/foo/baz > > In this case you end up with directory "foo" and at least two subdirs in > it, bar and baz. This works even if /tmp/foo existed previously and > even if there was some other stuff in it. Well, what about this, then? create schema if not exists foo create table second (a int); create schema if not exists foo create table second (b int); David
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > Excerpts from David E. Wheeler's message of mar oct 02 16:16:37 -0300 2012: >> On Oct 2, 2012, at 12:08 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >>> create schema if not exists foo create table first (a int); >>> create schema if not exists foo create table second (a int); >>> >>> As far as I can see, with the patch as it currently stands, you would >>> end up with only table "first" in the schema, which seems very >>> surprising to me. >> Yeah, I think the second should die. CINE should only work if there are no other objects created as part of the statement,IMHO. > Well, if that's the rationale then you end up with no schema foo at all > (i.e. both die), which seems even more surprising (though I admit it has > the advantage of being a simple rule to document.) I think we should just disallow putting any contained objects in the statement when IF NOT EXISTS is used. It's simple to understand, simple to document and implement, and I think it covers all the sane use-cases anyway. regards, tom lane
Excerpts from David E. Wheeler's message of mar oct 02 16:37:30 -0300 2012: > On Oct 2, 2012, at 12:30 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > > > How about call this for precedent: > > > > mkdir -p /tmp/foo/bar > > mkdir -p /tmp/foo/baz > > > > In this case you end up with directory "foo" and at least two subdirs in > > it, bar and baz. This works even if /tmp/foo existed previously and > > even if there was some other stuff in it. > > Well, what about this, then? > > create schema if not exists foo create table second (a int); > create schema if not exists foo create table second (b int); Yes, exactly -- what about this case? This is precisely the reason we don't have CREATE TABLE IF NOT EXISTS. I don't know what the best answer is. Most people seem to think that the answer ought to be that you end up with a single column second.a, and the second command errors out. So if you do this: create schema if not exists foo create table first (a int); create schema if not exists foo create table first (b int), create table second (a int); you end up with *only* the first table, because the second command errors out when the first table is observed to exist. Now, what if you were to do this instead: create schema if not exists foo create table if not exists first (a int); create schema if not exists foo create table if not exists first (b int), create table if not exists second (a int); The you end up with first.a and second.a. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Oct 2, 2012, at 12:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I think we should just disallow putting any contained objects in the > statement when IF NOT EXISTS is used. It's simple to understand, simple > to document and implement, and I think it covers all the sane use-cases > anyway. +1 David
On 10/02/2012 03:48 PM, Tom Lane wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: >> Excerpts from David E. Wheeler's message of mar oct 02 16:16:37 -0300 2012: >>> On Oct 2, 2012, at 12:08 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >>>> create schema if not exists foo create table first (a int); >>>> create schema if not exists foo create table second (a int); >>>> >>>> As far as I can see, with the patch as it currently stands, you would >>>> end up with only table "first" in the schema, which seems very >>>> surprising to me. >>> Yeah, I think the second should die. CINE should only work if there are no other objects created as part of the statement,IMHO. >> Well, if that's the rationale then you end up with no schema foo at all >> (i.e. both die), which seems even more surprising (though I admit it has >> the advantage of being a simple rule to document.) > I think we should just disallow putting any contained objects in the > statement when IF NOT EXISTS is used. It's simple to understand, simple > to document and implement, and I think it covers all the sane use-cases > anyway. > > I thought we'd already agreed on this. +1. cheers andrew
Excerpts from Andrew Dunstan's message of mar oct 02 17:24:38 -0300 2012: > > On 10/02/2012 03:48 PM, Tom Lane wrote: > > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > >> Well, if that's the rationale then you end up with no schema foo at all > >> (i.e. both die), which seems even more surprising (though I admit it has > >> the advantage of being a simple rule to document.) > > I think we should just disallow putting any contained objects in the > > statement when IF NOT EXISTS is used. It's simple to understand, simple > > to document and implement, and I think it covers all the sane use-cases > > anyway. > > I thought we'd already agreed on this. Well, it's not what the latest proposed patch implements. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
2012/10/2 Alvaro Herrera <alvherre@2ndquadrant.com>
-- Excerpts from Andrew Dunstan's message of mar oct 02 17:24:38 -0300 2012:> >> Well, if that's the rationale then you end up with no schema foo at allWell, it's not what the latest proposed patch implements.
> >> (i.e. both die), which seems even more surprising (though I admit it has
> >> the advantage of being a simple rule to document.)
> > I think we should just disallow putting any contained objects in the
> > statement when IF NOT EXISTS is used. It's simple to understand, simple
> > to document and implement, and I think it covers all the sane use-cases
> > anyway.
>
> I thought we'd already agreed on this.
You're right... the latest proposed patch don't implements it.
I'll change the patch and send soon...
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
This case points to a weakness in many programming languages, not having a clear ifof (if and only if) versus if construction.
The sane use case for create schema foo if not exists <object> is for building a database dynamically, where several points may be the first to put a table in a schema, and schemas should not be created if there are no objects. The create/search/drop design pattern having its own problems.
Thus the construction should default to one behavior, and have an option for the second.
e.g.
create schema foo if not exists (will not be done if foo existed)
create schema foo if not exists FORCE (will be done even if foo existed)
This would even allow for mixed e.g.
create schema foo if not exists (tables that should be created once and not again)
FORCE (objects routine will add if the schema does)
On Oct 2, 2012, at 6:33 PM, Fabrízio de Royes Mello wrote:
2012/10/2 Alvaro Herrera <alvherre@2ndquadrant.com>--Excerpts from Andrew Dunstan's message of mar oct 02 17:24:38 -0300 2012:> >> Well, if that's the rationale then you end up with no schema foo at allWell, it's not what the latest proposed patch implements.
> >> (i.e. both die), which seems even more surprising (though I admit it has
> >> the advantage of being a simple rule to document.)
> > I think we should just disallow putting any contained objects in the
> > statement when IF NOT EXISTS is used. It's simple to understand, simple
> > to document and implement, and I think it covers all the sane use-cases
> > anyway.
>
> I thought we'd already agreed on this.You're right... the latest proposed patch don't implements it.I'll change the patch and send soon...Regards,
Fabrízio de Royes MelloConsultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello>> Twitter: http://twitter.com/fabriziomello
2012/10/2 Fabrízio de Royes Mello <fabriziomello@gmail.com>
You're right... the latest proposed patch don't implements it.I'll change the patch and send soon...
* show a syntax error or
* show a message that you can not use the INE with contained objects
--
Fabrízio de Royes Mello
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Excerpts from Fabrízio de Royes Mello's message of mié oct 03 09:27:41 -0300 2012: > 2012/10/2 Fabrízio de Royes Mello <fabriziomello@gmail.com> > > > > > You're right... the latest proposed patch don't implements it. > > > > I'll change the patch and send soon... > > > > > What is more reasonable? > * show a syntax error or > * show a message that you can not use the INE with contained objects Second one. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
2012/10/3 Alvaro Herrera <alvherre@2ndquadrant.com>
Excerpts from Fabrízio de Royes Mello's message of mié oct 03 09:27:41 -0300 2012:> 2012/10/2 Fabrízio de Royes Mello <fabriziomello@gmail.com>Second one.
>
> >
> > You're right... the latest proposed patch don't implements it.
> >
> > I'll change the patch and send soon...
> >
> >
> What is more reasonable?
> * show a syntax error or
> * show a message that you can not use the INE with contained objects
Maybe something like this?
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("IF NOT EXISTS cannot be used with schema elements"),
parser_errposition(@9)));
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Excerpts from Fabrízio de Royes Mello's message of mié oct 03 10:11:03 -0300 2012: > Maybe something like this? > > ereport(ERROR, > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), > errmsg("IF NOT EXISTS cannot be used with schema elements"), > parser_errposition(@9))); Seems reasonable, but where? Please submit a complete patch. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
2012/10/3 Alvaro Herrera <alvherre@2ndquadrant.com>
Excerpts from Fabrízio de Royes Mello's message of mié oct 03 10:11:03 -0300 2012:Seems reasonable, but where? Please submit a complete patch.
> Maybe something like this?
>
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("IF NOT EXISTS cannot be used with schema elements"),
> parser_errposition(@9)));
The attached patch implements the behavior we've discussed.
If we use "IF NOT EXISTS" with schema elements then occurs an error like this:
[local]:5432 fabrizio@fabrizio=# CREATE SCHEMA IF NOT EXISTS test_schema_1
CREATE TABLE abc (
a serial,
b int UNIQUE
);
ERROR: IF NOT EXISTS cannot be used with schema elements
LINE 1: CREATE SCHEMA IF NOT EXISTS test_schema_1
^
Time: 0,773 ms
Regards,
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Attachment
Fabrízio de Royes Mello <fabriziomello@gmail.com> writes: > The attached patch implements the behavior we've discussed. OK, I'll pick this up again, since we seem to have consensus on this behavior. regards, tom lane
Fabrízio de Royes Mello <fabriziomello@gmail.com> writes: > The attached patch implements the behavior we've discussed. Committed with some adjustments, notably repairing the order-of-operations error I complained about before. regards, tom lane
On Oct 3, 2012, at 4:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Committed with some adjustments, notably repairing the > order-of-operations error I complained about before. Awesome, thanks! David