From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fail Fast In CTAS/CMV If Relation Already Exists To Avoid Unnecessary Rewrite, Planning Costs |
Date: | 2020-12-17 09:36:59 |
Message-ID: | CALj2ACXOm-3Yu=Sz7g2NKMEhPbyDfrFS2onVHohBzHFop+K-CQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Dec 14, 2020 at 1:54 PM Bharath Rupireddy <bharath.
rupireddyforpostgres(at)gmail(dot)com> wrote:
> On Mon, Dec 14, 2020 at 11:52 AM Michael Paquier <michael(at)paquier(dot)xyz>
wrote:
> > On Mon, Dec 14, 2020 at 03:15:12PM +0900, Michael Paquier wrote:
> > > Please note that this case fails with your patch, but the presence of
> > > IF NOT EXISTS should ensure that we don't fail and issue a NOTICE
> > > instead, no?
>
> Thanks for the use case. The provided use case (or for that matter any
> use case with explain analyze ctas if-not-exists) fails if the
> relation already exists. It happens on the master branch, please have
> a look at tests [1]. You are right in saying that whether it is
> explain/explain analyze ctas if there is if-not-exists we should issue
> notice instead of error as with plain ctas.
>
> Do we want to fix this behaviour for explain/explain analyze ctat with
> if-not-exists cases? Thoughts?
>
> If yes, we could change the code in ExplainOneUtility() such that we
> check relation existence before rewrite/planning, issue notice and
> return. Then. the user sees a notice and an empty plan as we are
> returning from ExplainOneUtility(). Is it okay to show a warning and
> an empty plan to the user? Thoughts?
>
> >> Taking this case specifically (OK, I am playing with
> > > the rules a bit to insert data into the relation itself, still), this
> > > query may finish by adding tuples to the table whose creation should
> > > have been bypassed but the query got executed and inserted tuples.
>
> IIUC, with the use case provided, the tuples will not be inserted as
> the query later fails (and the txn gets aborted) if the relation
> exists.
>
> > > That's one example of behavior that may be confusing. There may be
> > > others, but it seems to me that it may be simpler to execute or even
> > > plan the query at all if the relation already exists.
> >
> > Er.. Sorry. I meant here to *not* execute or even *not* plan the
> > query at all if the relation already exists.
>
> +1 to not plan and execute the query at all if the relation which
> ctas/cmv is trying to create already exists.
Posting a v2 patch after modifying the new function CheckRelExistenceInCTAS()
a bit as suggested earlier.
The behavior of the ctas/cmv, in case the relation already exists is as
shown in [1]. The things that have been changed with the patch are: 1) In
any case we do not rewrite or plan the select part if the relation already
exists 2) For explain ctas/cmv (without analyze), now the relation
existence is checked early and the error is thrown as highlighted in [1].
With patch, there is no behavioral change(from that of master branch) in
explain analyze ctas/cmv with if-not-exists i.e. error is thrown not the
notice.
Thoughts?
[1]
With patch:
postgres=# create table foo as select 1;
ERROR: relation "foo" already exists
postgres=# create table if not exists foo as select 1;
NOTICE: relation "foo" already exists, skipping
CREATE TABLE AS
postgres=# explain analyze create table foo as select 1;
ERROR: relation "foo" already exists
postgres=# explain analyze create table if not exists foo as select 1;
ERROR: relation "foo" already exists
postgres=# explain create table foo as select 1;
ERROR: relation "foo" already exists
postgres=# explain create table if not exists foo as select 1;
ERROR: relation "foo" already exists
On master/without patch:
postgres=# create table foo as select 1;
ERROR: relation "foo" already exists
postgres=# create table if not exists foo as select 1;
NOTICE: relation "foo" already exists, skipping
CREATE TABLE AS
postgres=# explain analyze create table foo as select 1;
ERROR: relation "foo" already exists
postgres=# explain analyze create table if not exists foo as select 1;
ERROR: relation "foo" already exists
postgres=# explain create table foo as select 1;
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.01 rows=1 width=4)
(1 row)
postgres=# explain create table if not exists foo as select 1;
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.01 rows=1 width=4)
(1 row)
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Fail-Fast-In-CTAS-CMV-If-Relation-Already-Exists.patch | application/x-patch | 5.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2020-12-17 09:45:00 | Re: STANDBY_LOCK_TIMEOUT may not interrupt ProcWaitForSignal()? |
Previous Message | Andy Fan | 2020-12-17 09:22:30 | Re: Cache relation sizes? |