Re: Declarative partitioning - another take

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