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