CREATE IF NOT EXISTS INDEX

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
37 messages Options
12
Reply | Threaded
Open this post in threaded view
|

CREATE IF NOT EXISTS INDEX

fabriziomello
Hi all,

What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX? As it holds data (like sequences and tables) I think we can do that.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Tom Lane-2
=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
> What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?

It's got the same semantic problems as every other variant of CINE.

If there were a huge groundswell of demand for it, maybe we'd hold our
noses and do it anyway.  But I'm against doing it without that.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

bithead

Since PostgreSQL started down that road for so many other relations, I think many people just expect this to happen as a logical extension.

It certainly makes life a lot easier in combination with build management systems.

/kirk

 

On 2014-09-30 16:43, Tom Lane wrote:

=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?
It's got the same semantic problems as every other variant of CINE.

If there were a huge groundswell of demand for it, maybe we'd hold our
noses and do it anyway.  But I'm against doing it without that.

			regards, tom lane

 

Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Josh berkus
In reply to this post by Tom Lane-2
On 09/30/2014 02:43 PM, Tom Lane wrote:
> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
>> What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?
>
> It's got the same semantic problems as every other variant of CINE.
>
> If there were a huge groundswell of demand for it, maybe we'd hold our
> noses and do it anyway.  But I'm against doing it without that.

This isn't the sort of thing there would ever be a clamor of support
for, because it's just not that visible of a feature.  It's more of a
regular annoyance for those who encounter it.  More importantly, adding
an IF NOT EXISTS to CREATE INDEX would allow complete idempotent "create
this bunch of tables" scripts, since now the "create index" statements
could be included.  This would be very nice for schema management tools.

I do think it should be name-based.  While an "IF NOT EXISTS" which
checked for a duplicate column declartion would be nice, there's a raft
of issues with implementing it that way.  Users I know are generally
just looking to avoid getting a transaction-halting error when they run
the same create index statement twice.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

bithead

On 2014-09-30 17:01, Josh Berkus wrote:

On 09/30/2014 02:43 PM, Tom Lane wrote:
=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?
It's got the same semantic problems as every other variant of CINE. If there were a huge groundswell of demand for it, maybe we'd hold our noses and do it anyway. But I'm against doing it without that.
This isn't the sort of thing there would ever be a clamor of support
for, because it's just not that visible of a feature.  It's more of a
regular annoyance for those who encounter it.  More importantly, adding
an IF NOT EXISTS to CREATE INDEX would allow complete idempotent "create
this bunch of tables" scripts, since now the "create index" statements
could be included.  This would be very nice for schema management tools.

I do think it should be name-based.  While an "IF NOT EXISTS" which
checked for a duplicate column declartion would be nice, there's a raft
of issues with implementing it that way.  Users I know are generally
just looking to avoid getting a transaction-halting error when they run
the same create index statement twice.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

I second this evaluation.   Using build tools to manage schemas, there is no expectation that the full index signature is checked.  Any index of the same name would be considered a collision for my purposes.

It is much easier to show CINE to a developer than to explain how an anonymous code block does the same thing.

/Kirk

Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

fabriziomello
In reply to this post by Josh berkus


On Tue, Sep 30, 2014 at 7:01 PM, Josh Berkus <[hidden email]> wrote:

>
> On 09/30/2014 02:43 PM, Tom Lane wrote:
> > =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
> >> What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?
> >
> > It's got the same semantic problems as every other variant of CINE.
> >
> > If there were a huge groundswell of demand for it, maybe we'd hold our
> > noses and do it anyway.  But I'm against doing it without that.
>
> This isn't the sort of thing there would ever be a clamor of support
> for, because it's just not that visible of a feature.  It's more of a
> regular annoyance for those who encounter it.  More importantly, adding
> an IF NOT EXISTS to CREATE INDEX would allow complete idempotent "create
> this bunch of tables" scripts, since now the "create index" statements
> could be included.  This would be very nice for schema management tools.
>
> I do think it should be name-based.  While an "IF NOT EXISTS" which
> checked for a duplicate column declartion would be nice, there's a raft
> of issues with implementing it that way.  Users I know are generally
> just looking to avoid getting a transaction-halting error when they run
> the same create index statement twice.
>

Here is the patch... it's name-based.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

create_index_if_not_exists_v1.patch (14K) Download Attachment
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Tom Lane-2
In reply to this post by Josh berkus
Josh Berkus <[hidden email]> writes:
> On 09/30/2014 02:43 PM, Tom Lane wrote:
>> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
>>> What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?

>> It's got the same semantic problems as every other variant of CINE.

> I do think it should be name-based.

Name-based, eh?  Don't you recall that in modern practice, people
generally don't specify names for indexes at all?  They've usually
got system-generated names, which doesn't seem like a very cool thing
to have scripts depending on.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Andres Freund-3
On 2014-09-30 18:47:24 -0400, Tom Lane wrote:

> Josh Berkus <[hidden email]> writes:
> > On 09/30/2014 02:43 PM, Tom Lane wrote:
> >> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
> >>> What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?
>
> >> It's got the same semantic problems as every other variant of CINE.
>
> > I do think it should be name-based.
>
> Name-based, eh?  Don't you recall that in modern practice, people
> generally don't specify names for indexes at all?  They've usually
> got system-generated names, which doesn't seem like a very cool thing
> to have scripts depending on.

Good point. I think it's fair enough to only allow CINE on named
indexes.

Greetings,

Andres Freund

--
 Andres Freund                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Josh berkus
On 09/30/2014 03:53 PM, Andres Freund wrote:

> On 2014-09-30 18:47:24 -0400, Tom Lane wrote:
>> Josh Berkus <[hidden email]> writes:
>>> On 09/30/2014 02:43 PM, Tom Lane wrote:
>>>> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
>>>>> What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?
>>
>>>> It's got the same semantic problems as every other variant of CINE.
>>
>>> I do think it should be name-based.
>>
>> Name-based, eh?  Don't you recall that in modern practice, people
>> generally don't specify names for indexes at all?  They've usually
>> got system-generated names, which doesn't seem like a very cool thing
>> to have scripts depending on.
>
> Good point. I think it's fair enough to only allow CINE on named
> indexes.

On the other hand, the way we form system-generated names is predicable,
so I think it would be perfectly OK to include them.  Desirable, in fact.

For example, if I did this:

CREATE INDEX ON tab1 (cola, colb);

CREATE INDEX IF NOT EXISTS ON tab1 (cola, colb);

I would expect to not end up with two indexes on those two particular
columns, and if we don't omit system-generated names, I won't.

Not that I'm a fan of omitting the name ...

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Andres Freund-3
On 2014-09-30 16:03:01 -0700, Josh Berkus wrote:

> On 09/30/2014 03:53 PM, Andres Freund wrote:
> > On 2014-09-30 18:47:24 -0400, Tom Lane wrote:
> >> Josh Berkus <[hidden email]> writes:
> >>> On 09/30/2014 02:43 PM, Tom Lane wrote:
> >>>> =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <[hidden email]> writes:
> >>>>> What's your thoughts about we implement IF NOT EXISTS for CREATE INDEX?
> >>
> >>>> It's got the same semantic problems as every other variant of CINE.
> >>
> >>> I do think it should be name-based.
> >>
> >> Name-based, eh?  Don't you recall that in modern practice, people
> >> generally don't specify names for indexes at all?  They've usually
> >> got system-generated names, which doesn't seem like a very cool thing
> >> to have scripts depending on.
> >
> > Good point. I think it's fair enough to only allow CINE on named
> > indexes.
>
> On the other hand, the way we form system-generated names is predicable,
> so I think it would be perfectly OK to include them.  Desirable, in fact.

It's not really that predicable. Think about expression indexes. They
also don't contain information about opclasses et all.

Seems like pit of hairy semantics.

> Not that I'm a fan of omitting the name ...

Me neither.

Greetings,

Andres Freund

--
 Andres Freund                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Josh berkus
On 09/30/2014 04:16 PM, Andres Freund wrote:

> On 2014-09-30 16:03:01 -0700, Josh Berkus wrote:
>> On 09/30/2014 03:53 PM, Andres Freund wrote:
>>> Good point. I think it's fair enough to only allow CINE on named
>>> indexes.
>>
>> On the other hand, the way we form system-generated names is predicable,
>> so I think it would be perfectly OK to include them.  Desirable, in fact.
>
> It's not really that predicable. Think about expression indexes. They
> also don't contain information about opclasses et all.
>
> Seems like pit of hairy semantics.
>
>> Not that I'm a fan of omitting the name ...
>
> Me neither.

I'd be OK with a CINE which required you to name the index.

How does your patch work at present, Fabrizio?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

fabriziomello


On Tue, Sep 30, 2014 at 8:47 PM, Josh Berkus <[hidden email]> wrote:

>
> On 09/30/2014 04:16 PM, Andres Freund wrote:
> > On 2014-09-30 16:03:01 -0700, Josh Berkus wrote:
> >> On 09/30/2014 03:53 PM, Andres Freund wrote:
> >>> Good point. I think it's fair enough to only allow CINE on named
> >>> indexes.
> >>
> >> On the other hand, the way we form system-generated names is predicable,
> >> so I think it would be perfectly OK to include them.  Desirable, in fact.
> >
> > It's not really that predicable. Think about expression indexes. They
> > also don't contain information about opclasses et all.
> >
> > Seems like pit of hairy semantics.
> >
> >> Not that I'm a fan of omitting the name ...
> >
> > Me neither.
>
> I'd be OK with a CINE which required you to name the index.
>
> How does your patch work at present, Fabrizio?
>

My patch will work just if you name the index, because postgres generates a index name that doesn't exists.

I don't check to a name if we use IF NOT EXISTS, but I can add this check.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Josh berkus
On 09/30/2014 04:58 PM, Fabrízio de Royes Mello wrote:

> On Tue, Sep 30, 2014 at 8:47 PM, Josh Berkus <[hidden email]> wrote:
>>
>> On 09/30/2014 04:16 PM, Andres Freund wrote:
>>> On 2014-09-30 16:03:01 -0700, Josh Berkus wrote:
>>>> On 09/30/2014 03:53 PM, Andres Freund wrote:
>>>>> Good point. I think it's fair enough to only allow CINE on named
>>>>> indexes.
>>>>
>>>> On the other hand, the way we form system-generated names is
> predicable,
>>>> so I think it would be perfectly OK to include them.  Desirable, in
> fact.
>>>
>>> It's not really that predicable. Think about expression indexes. They
>>> also don't contain information about opclasses et all.
>>>
>>> Seems like pit of hairy semantics.
>>>
>>>> Not that I'm a fan of omitting the name ...
>>>
>>> Me neither.
>>
>> I'd be OK with a CINE which required you to name the index.
>>
>> How does your patch work at present, Fabrizio?
>>
>
> My patch will work just if you name the index, because postgres generates a
> index name that doesn't exists.
>
> I don't check to a name if we use IF NOT EXISTS, but I can add this check.

The consensus is that we don't want IF NOT EXISTS to work for
automatically generated index names.  For that reason, we'd want it to
error out if someone does this:

CREATE INDEX IF NOT EXISTS ON table(col);

My suggestion for the error message:

"IF NOT EXISTS requires that you name the index."

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

fabriziomello

On Tue, Sep 30, 2014 at 9:12 PM, Josh Berkus <[hidden email]> wrote:

>
> On 09/30/2014 04:58 PM, Fabrízio de Royes Mello wrote:
> > On Tue, Sep 30, 2014 at 8:47 PM, Josh Berkus <[hidden email]> wrote:
> >>
> >> On 09/30/2014 04:16 PM, Andres Freund wrote:
> >>> On 2014-09-30 16:03:01 -0700, Josh Berkus wrote:
> >>>> On 09/30/2014 03:53 PM, Andres Freund wrote:
> >>>>> Good point. I think it's fair enough to only allow CINE on named
> >>>>> indexes.
> >>>>
> >>>> On the other hand, the way we form system-generated names is
> > predicable,
> >>>> so I think it would be perfectly OK to include them.  Desirable, in
> > fact.
> >>>
> >>> It's not really that predicable. Think about expression indexes. They
> >>> also don't contain information about opclasses et all.
> >>>
> >>> Seems like pit of hairy semantics.
> >>>
> >>>> Not that I'm a fan of omitting the name ...
> >>>
> >>> Me neither.
> >>
> >> I'd be OK with a CINE which required you to name the index.
> >>
> >> How does your patch work at present, Fabrizio?
> >>
> >
> > My patch will work just if you name the index, because postgres generates a
> > index name that doesn't exists.
> >
> > I don't check to a name if we use IF NOT EXISTS, but I can add this check.
>
> The consensus is that we don't want IF NOT EXISTS to work for
> automatically generated index names.  For that reason, we'd want it to
> error out if someone does this:
>
> CREATE INDEX IF NOT EXISTS ON table(col);
>
> My suggestion for the error message:
>
> "IF NOT EXISTS requires that you name the index."
>

Done.

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

create_index_if_not_exists_v2.patch (15K) Download Attachment
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Michael Paquier
On Wed, Oct 1, 2014 at 10:03 AM, Fabrízio de Royes Mello <[hidden email]> wrote:
Done.
You should consider adding that to the next commit fest.
--
Michael
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

fabriziomello

On Tue, Sep 30, 2014 at 10:22 PM, Michael Paquier <[hidden email]> wrote:
>
> On Wed, Oct 1, 2014 at 10:03 AM, Fabrízio de Royes Mello <[hidden email]> wrote:
>>
>> Done.
>
> You should consider adding that to the next commit fest.
>

Sure. Added [1]

Regards,

[1] https://commitfest.postgresql.org/action/patch_view?id=1584

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

fabriziomello
In reply to this post by fabriziomello

On Wed, Oct 1, 2014 at 7:57 AM, José Luis Tallón <[hidden email]> wrote:

>
> [snip]
>
> Please excuse my jumping in, but the EXPECTED syntax is:
>
> CREATE INDEX IF NOT EXISTS .....
>
> whereas your current patch implements:
>
> CREATE [IF NOT EXISTS] INDEX ....
>
>
> if I'm reading the grammar correctly.
>

I think it's not wrong. Look at other CINE that already implemented [1] [2].

But CINE for CREATE TABLE is like your proposal [3] :

CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXISTS ] table_name ...

So, what's the correct/best grammar?

CREATE [ IF NOT EXISTS ] [ UNIQUE ] INDEX index_name

or

CREATE [ UNIQUE ] INDEX [ IF NOT EXISTS ] index_name


> I guess it would be most interesting to implement this minor change for the next version of the patch. Please do remember to update the documentation accordingly.
>

I will...


> By the way, you also forgot to remove a previous patch implementing "namespace_name<DOT>relation_name" for RLS messages. Maybe a rebase is needed?
>

Sorry... my mistake. Fix attached.



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

create_index_if_not_exists_v3.patch (14K) Download Attachment
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Marti Raudsepp
On Wed, Oct 1, 2014 at 2:42 PM, Fabrízio de Royes Mello
<[hidden email]> wrote:
> So, what's the correct/best grammar?
> CREATE [ IF NOT EXISTS ] [ UNIQUE ] INDEX index_name
> or
> CREATE [ UNIQUE ] INDEX [ IF NOT EXISTS ] index_name

I've elected myself as the reviewer for this patch. Here are some
preliminary comments...

I agree with José. The 2nd is more consistent given the other syntaxes:
  CREATE { TABLE | SCHEMA | EXTENSION | ... } IF NOT EXISTS name ...
It's also compatible with SQLite's grammar:
https://www.sqlite.org/lang_createindex.html

Do we want to enforce an order on the keywords or allow both?
  CREATE INDEX IF NOT EXISTS CONCURRENTLY foo ...
  CREATE INDEX CONCURRENTLY IF NOT EXISTS foo ...

It's probably very rare to use both keywords at the same time, so I'd
prefer only the 2nd, unless someone else chimes in.

Documentation: I would prefer if the explanation were consistent with
the description for ALTER TABLE/EXTENSION; just copy it and replace
"relation" with "index".

+ ereport(NOTICE,
+                 (errcode(ERRCODE_DUPLICATE_TABLE),
+                  errmsg("relation \"%s\" already exists, skipping",
+                                 indexRelationName)));

1. Clearly "relation" should be "index".
2. Use ERRCODE_DUPLICATE_OBJECT not TABLE

+ if (n->if_not_exists && n->idxname == NULL)
+         ereport(ERROR,
+                         (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                          errmsg("IF NOT EXISTS requires that you
name the index."),

I think ERRCODE_SYNTAX_ERROR makes more sense, it's something that we
decided we *don't want* to support.

- write_msg(NULL, "reading row-security enabled for table \"%s\"",
+ write_msg(NULL, "reading row-security enabled for table \"%s\"\n",

???

Regards,
Marti


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

Marti Raudsepp
On Fri, Oct 3, 2014 at 2:15 AM, Marti Raudsepp <[hidden email]> wrote:
> + ereport(NOTICE,
> +                 (errcode(ERRCODE_DUPLICATE_TABLE),
> +                  errmsg("relation \"%s\" already exists, skipping",
> +                                 indexRelationName)));
>
> 1. Clearly "relation" should be "index".
> 2. Use ERRCODE_DUPLICATE_OBJECT not TABLE

My bad, this code is OK. The current code already uses "relation" and
TABLE elsewhere because indexes share the same namespace with tables.

+ /*
+  * Throw an exception when IF NOT EXISTS is used without a named
+  * index
+  */

I'd say "without an index name". And the line goes beyond 80 characters wide.

I would also move this check to after all the attributes have been
assigned, rather than splitting the assignments in half.

Regards,
Marti


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: CREATE IF NOT EXISTS INDEX

fabriziomello
In reply to this post by Marti Raudsepp
On Thu, Oct 2, 2014 at 8:15 PM, Marti Raudsepp <[hidden email]> wrote:

>
> On Wed, Oct 1, 2014 at 2:42 PM, Fabrízio de Royes Mello
> <[hidden email]> wrote:
> > So, what's the correct/best grammar?
> > CREATE [ IF NOT EXISTS ] [ UNIQUE ] INDEX index_name
> > or
> > CREATE [ UNIQUE ] INDEX [ IF NOT EXISTS ] index_name
>
> I've elected myself as the reviewer for this patch. Here are some
> preliminary comments...
>

Thanks...


> I agree with José. The 2nd is more consistent given the other syntaxes:
>   CREATE { TABLE | SCHEMA | EXTENSION | ... } IF NOT EXISTS name ...
> It's also compatible with SQLite's grammar:
> https://www.sqlite.org/lang_createindex.html
>
> Do we want to enforce an order on the keywords or allow both?
>   CREATE INDEX IF NOT EXISTS CONCURRENTLY foo ...
>   CREATE INDEX CONCURRENTLY IF NOT EXISTS foo ...
>
> It's probably very rare to use both keywords at the same time, so I'd
> prefer only the 2nd, unless someone else chimes in.
>

Fixed.


> Documentation: I would prefer if the explanation were consistent with
> the description for ALTER TABLE/EXTENSION; just copy it and replace
> "relation" with "index".
>

Well, I'm not native English so I tend to agree with you.

"Do not throw an error if the index already exists. A notice is issued in this case."

Fixed in that way. Ok?


> + ereport(NOTICE,
> +                 (errcode(ERRCODE_DUPLICATE_TABLE),
> +                  errmsg("relation \"%s\" already exists, skipping",
> +                                 indexRelationName)));
>
> 1. Clearly "relation" should be "index".
> 2. Use ERRCODE_DUPLICATE_OBJECT not TABLE
>

Sorry, but I'm not with you. I just copy the original error message and add ", skipping" at the end.

 788         ereport(ERROR,
 789                 (errcode(ERRCODE_DUPLICATE_TABLE),
 790                  errmsg("relation \"%s\" already exists",
 791                         indexRelationName)));


> + if (n->if_not_exists && n->idxname == NULL)
> +         ereport(ERROR,
> +                         (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +                          errmsg("IF NOT EXISTS requires that you
> name the index."),
>
> I think ERRCODE_SYNTAX_ERROR makes more sense, it's something that we
> decided we *don't want* to support.
>

I don't think so. It's the same as CREATE SCHEMA IF NOT EXISTS that not support to include schema elements.

Other opinions?


> - write_msg(NULL, "reading row-security enabled for table \"%s\"",
> + write_msg(NULL, "reading row-security enabled for table \"%s\"\n",
>
> ???
>

Sorry, my mistake. I merged with a wrong local branch. Fixed.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

create_index_if_not_exists_v4.patch (14K) Download Attachment
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
12