Subject: Re: Getting ERROR: could not open file "base/13164/t3_16388" with
 partition table with ON COMMIT
References: 
 <CAKcux6mhgcjSiB_egqEAEFgX462QZtncU8QCAJ2HZwM-wWGVew@mail.gmail.com>
 <2bf804fb-7417-4539-51c5-f6419f1e8741@lab.ntt.co.jp>
 <7193.1536768840@sss.pgh.pa.us>
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Message-ID: <e1b7c3ec-dbfc-4295-8bec-ee985114ecb5@lab.ntt.co.jp>
Date: Thu, 13 Sep 2018 13:36:22 +0900
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <7193.1536768840@sss.pgh.pa.us>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Precedence: bulk

On 2018/09/13 1:14, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> The infamous missing-relkind-check in heap_truncate() seems to be behind
>> this.  Perhaps, a patch like the attached will do?
> 
> That seems excessively restrictive.  Anything that has storage (e.g.
> matviews) ought to be truncatable, no?

Not by heap_truncate it seems.  The header comment of heap_truncate says
that it concerns itself only with ON COMMIT truncation of temporary tables:

/*
 *   heap_truncate
 *
 *   This routine deletes all data within all the specified relations.
 *
 * This is not transaction-safe!  There is another, transaction-safe
 * implementation in commands/tablecmds.c.  We now use this only for
 * ON COMMIT truncation of temporary tables, where it doesn't matter.
 */

ON COMMIT clause can only be used with temporary tables, so the only two
possible relkind values that can be encountered here are RELKIND_RELATION
and RELKIND_PARTITIONED_TABLE.  Of the two, only the RELKIND_RELATION can
have storage.

> I thought we had a macro or utility function somewhere that knew which
> relkinds have storage, though I can't find it right now.  I'd be
> inclined to instantiate that if it doesn't exist, and then the code
> here ought to read something like
> 
> 	if (RelkindHasStorage(rel->rd_rel->relkind))
> 		heap_truncate_one_rel(rel);

There have been discussions (such as [1]), but none that led to some patch
being committed.  Might be a good idea to revive that discussion again, or
perhaps there is already some solution being discussed on the pluggable
storage thread.

> Also, possibly the test ought to be inside heap_truncate_one_rel
> rather than its callers?

Hmm, perhaps.  ExecuteTruncateGuts, the only other caller of
heap_truncate_one_rel, also checks the relkind to skip partitioned tables.
 There seem to be reasons to do it in the caller in that case though,
other than heap_truncate_one_rel being incapable of handling them.

Thanks,
Amit


[1] Macros bundling RELKIND_* conditions
https://www.postgresql.org/message-id/CAFjFpRcfzs%2Byst6YBCseD_orEcDNuAr9GUTraZ5GC%3DAvCYh55Q%40mail.gmail.com