Thread: patch: ALTER TABLE IF EXISTS
Hello this is relative simple patch that add possibility to skip noexisting tables. It is necessary for silent cleaning when dump is loaded. Regards Pavel Stehule
Attachment
On Mon, Jan 2, 2012 at 1:11 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > this is relative simple patch that add possibility to skip noexisting > tables. It is necessary for silent cleaning when dump is loaded. Agreed, nice simple and uncontentious patch. All good apart from two minor things: * doc page needs to explain what IF EXISTS does, like DROP TABLE, e.g. IF EXISTS Do not throw an error if the table does not exist. A notice is issued in this case. * the error message is "does not exist" rather than "does not exists", which means the code, a comment and a test need changing Other than that, happy to apply as soon as you make those changes. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 2 Leden 2012, 14:11, Pavel Stehule wrote: > Hello > > this is relative simple patch that add possibility to skip noexisting > tables. It is necessary for silent cleaning when dump is loaded. Just a curious question - what use case is solved by this? Under what circumstances you get an ALTER TABLE without a preceding CREATE TABLE? I can't think of such scenario ... Or is this meant for scripts written manually so that it's possible to do alter if the table already exists and create if it does not exist? Tomas
Hello 2012/1/2 Tomas Vondra <tv@fuzzy.cz>: > On 2 Leden 2012, 14:11, Pavel Stehule wrote: >> Hello >> >> this is relative simple patch that add possibility to skip noexisting >> tables. It is necessary for silent cleaning when dump is loaded. > > Just a curious question - what use case is solved by this? Under what > circumstances you get an ALTER TABLE without a preceding CREATE TABLE? I > can't think of such scenario ... > > Or is this meant for scripts written manually so that it's possible to do > alter if the table already exists and create if it does not exist? this is necessary for "silent" cleaning in pg_dump this is fragment of dump with -c option ALTER TABLE ONLY public.b DROP CONSTRAINT b_b_fkey; DROP INDEX public.a_a_idx; ALTER TABLE ONLY public.a DROP CONSTRAINT a_pkey; DROP TABLE public.b; DROP TABLE public.a; DROP EXTENSION plpgsql; DROP SCHEMA public; I am working on "silent cleaning" and I am able generate a sequence of statements: ALTER TABLE IF EXISTS ONLY public.b DROP CONSTRAINT b_b_fkey; DROP INDEX IF EXISTS public.a_a_idx; ALTER TABLE IF EXISTS ONLY public.a DROP CONSTRAINT a_pkey; DROP TABLE IF EXISTS public.b; DROP TABLE IF EXISTS public.a; DROP EXTENSION IF EXISTS plpgsql; DROP SCHEMA IF EXISTS public; constraint b_b_fkey should be removed before dropping index a_a_idx Now we have DROP .. IF EXISTS statements, but ALTER TABLE .. IF EXISTS missing Regards Pavel > > Tomas >
Hello here is updated patch Regards Pavel 2012/1/2 Simon Riggs <simon@2ndquadrant.com>: > On Mon, Jan 2, 2012 at 1:11 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > >> this is relative simple patch that add possibility to skip noexisting >> tables. It is necessary for silent cleaning when dump is loaded. > > Agreed, nice simple and uncontentious patch. > > All good apart from two minor things: > > * doc page needs to explain what IF EXISTS does, like DROP TABLE, e.g. > > IF EXISTS > Do not throw an error if the table does not exist. A notice is > issued in this case. > > * the error message is "does not exist" rather than "does not exists", > which means the code, a comment and a test need changing > > Other than that, happy to apply as soon as you make those changes. > > -- > Simon Riggs http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services
Attachment
On Mon, Jan 2, 2012 at 12:01 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > here is updated patch I think the comments in parse_utilcmd.c probably need a bit of adjustment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hello 2012/1/3 Robert Haas <robertmhaas@gmail.com>: > On Mon, Jan 2, 2012 at 12:01 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> here is updated patch > > I think the comments in parse_utilcmd.c probably need a bit of adjustment. I don't see it - there is only one comment and it is adjusted with "if" statement. please, show it Regards Pavel > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company
On Tue, Jan 3, 2012 at 10:38 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
> 2012/1/3 Robert Haas <robertmhaas@gmail.com>:
>> On Mon, Jan 2, 2012 at 12:01 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>> here is updated patch
>>
>> I think the comments in parse_utilcmd.c probably need a bit of adjustment.
>
> I don't see it - there is only one comment and it is adjusted with
> "if" statement.
>
> please, show it
Well, basically, the comment preceding the part you altered say "the
lock level requested here", but "here" is getting spread out quite a
bit more with this code change. Maybe that doesn't matter.
However, on further examination, this is a pretty awkward way to write
the code. Why not something like this:
rel = relation_openrv_extended(stmt->relation, lockmode, stmt->missing_ok);
if (rel == NULL)
{ ereport(...); return NIL;
}
Maybe the intent of sticking heap_openrv_extended() into the upper
branch of the if statement is to try to bounce relations that aren't
tables, but that's not actually what that function does (e.g. a
foreign table will slip through). And I think if we're going to have
IF EXISTS support for ALTER TABLE at all, we ought to have it for the
whole family of related statements: ALTER VIEW, ALTER SEQUENCE, ALTER
FOREIGN TABLE, etc., not just ALTER TABLE itself.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hello
2012/1/3 Robert Haas <robertmhaas@gmail.com>:
> On Tue, Jan 3, 2012 at 10:38 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Hello
>>
>> 2012/1/3 Robert Haas <robertmhaas@gmail.com>:
>>> On Mon, Jan 2, 2012 at 12:01 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>>> here is updated patch
>>>
>>> I think the comments in parse_utilcmd.c probably need a bit of adjustment.
>>
>> I don't see it - there is only one comment and it is adjusted with
>> "if" statement.
>>
>> please, show it
>
> Well, basically, the comment preceding the part you altered say "the
> lock level requested here", but "here" is getting spread out quite a
> bit more with this code change. Maybe that doesn't matter.
>
> However, on further examination, this is a pretty awkward way to write
> the code. Why not something like this:
>
> rel = relation_openrv_extended(stmt->relation, lockmode, stmt->missing_ok);
> if (rel == NULL)
> {
> ereport(...);
> return NIL;
> }
>
> Maybe the intent of sticking heap_openrv_extended() into the upper
> branch of the if statement is to try to bounce relations that aren't
> tables, but that's not actually what that function does (e.g. a
> foreign table will slip through). And I think if we're going to have
> IF EXISTS support for ALTER TABLE at all, we ought to have it for the
> whole family of related statements: ALTER VIEW, ALTER SEQUENCE, ALTER
> FOREIGN TABLE, etc., not just ALTER TABLE itself.
>
jup, we can continue in enhancing step by step.
I change a patch and now ALTER TABLE, ALTER INDEX, ALTER SEQUENCE and
ALTER VIEW has IF EXISTS clause
Regards
Pavel
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
Attachment
On Tue, Jan 3, 2012 at 7:49 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > I change a patch and now ALTER TABLE, ALTER INDEX, ALTER SEQUENCE and > ALTER VIEW has IF EXISTS clause Patch no longer applies. Pls update. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Tue, Jan 3, 2012 at 2:49 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > jup, we can continue in enhancing step by step. > > I change a patch and now ALTER TABLE, ALTER INDEX, ALTER SEQUENCE and > ALTER VIEW has IF EXISTS clause ALTER FOREIGN TABLE should be parallel as well, I think. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hello 2012/1/23 Robert Haas <robertmhaas@gmail.com>: > On Tue, Jan 3, 2012 at 2:49 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> jup, we can continue in enhancing step by step. >> >> I change a patch and now ALTER TABLE, ALTER INDEX, ALTER SEQUENCE and >> ALTER VIEW has IF EXISTS clause > > ALTER FOREIGN TABLE should be parallel as well, I think. > refreshed + ALTER FOREIGN TABLE IF EXISTS ... support Regards Pavel > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company
Attachment
On 23 January 2012 20:14, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
> 2012/1/23 Robert Haas <robertmhaas@gmail.com>:
>> On Tue, Jan 3, 2012 at 2:49 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>> jup, we can continue in enhancing step by step.
>>>
>>> I change a patch and now ALTER TABLE, ALTER INDEX, ALTER SEQUENCE and
>>> ALTER VIEW has IF EXISTS clause
>>
>> ALTER FOREIGN TABLE should be parallel as well, I think.
>>
>
> refreshed + ALTER FOREIGN TABLE IF EXISTS ... support
>
I just noticed this copy-and-paste error in the ALTER FOREIGN TABLE docs:
IF EXISTS:
Do not throw an error if the sequence does not exist. A notice is issued in this case.
That should be "foreign table" not "sequence".
Regards,
Dean
On 27.01.2012 11:57, Dean Rasheed wrote: > I just noticed this copy-and-paste error in the ALTER FOREIGN TABLE docs: > > IF EXISTS: > > Do not throw an error if the sequence does not exist. A notice is issued > in this case. > > That should be "foreign table" not "sequence". Thanks, fixed. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com