Re: Declarative partitioning - another take

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Dmitry Ivanov <d(dot)ivanov(at)postgrespro(dot)ru>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Erik Rijkers <er(at)xs4all(dot)nl>, Amit Langote <amitlangote09(at)gmail(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers-owner(at)postgresql(dot)org
Subject: Re: Declarative partitioning - another take
Date: 2016-12-16 09:04:20
Message-ID: e8d20f5a-a847-d644-be44-210174ed88b4@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Dmitry,

On 2016/12/16 0:40, Dmitry Ivanov wrote:
> Hi everyone,
>
> Looks like "sql_inheritance" GUC is affecting partitioned tables:
>
> explain (costs off) select * from test;
> QUERY PLAN ------------------------------
> Append
> -> Seq Scan on test
> -> Seq Scan on test_1
> -> Seq Scan on test_2
> -> Seq Scan on test_1_1
> -> Seq Scan on test_1_2
> -> Seq Scan on test_1_1_1
> -> Seq Scan on test_1_2_1
> (8 rows)
>
>
> set sql_inheritance = off;
>
>
> explain (costs off) select * from test;
> QUERY PLAN ------------------
> Seq Scan on test
> (1 row)
>
>
> I might be wrong, but IMO this should not happen. Queries involving
> update, delete etc on partitioned tables are basically broken. Moreover,
> there's no point in performing such operations on a parent table that's
> supposed to be empty at all times.
>
> I've come up with a patch which fixes this behavior for UPDATE, DELETE,
> TRUNCATE and also in transformTableEntry(). It might be hacky, but it
> gives an idea.
>
> I didn't touch RenameConstraint() and renameatt() since this would break
> ALTER TABLE ONLY command.

@@ -1198,6 +1198,12 @@ ExecuteTruncate(TruncateStmt *stmt)
rels = lappend(rels, rel);
relids = lappend_oid(relids, myrelid);

+ /* Use interpretInhOption() unless it's a partitioned table */
+ if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+ recurse = interpretInhOption(rv->inhOpt);
+ else
+ recurse = true;
+
if (recurse)
{
ListCell *child;

If you see the else block of this if, you'll notice this:

else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("must truncate child tables too")));

So that you get this behavior:

# set sql_inheritance to off;
SET

# truncate p;
ERROR: must truncate child tables too

# reset sql_inheritance;
RESET

# truncate only p;
ERROR: must truncate child tables too

# truncate p;
TRUNCATE TABLE

Beside that, I initially had implemented the same thing as what you are
proposing here, but reverted to existing behavior at some point during the
discussion. I think the idea behind was to not *silently* ignore user
specified configuration and instead error out with appropriate message.
While it seems to work reasonably for DDL and maintenance commands (like
TRUNCATE above), things sound strange for SELECT/UPDATE/DELETE as you're
saying.

Thanks,
Amit

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Matteo Beccati 2016-12-16 09:08:43 Re: jsonb problematic operators
Previous Message Amit Langote 2016-12-16 08:58:04 Re: Declarative partitioning - another take