Horton

Re: unsupportable composite type partition keys

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2019-12-21T21:13:04Z
Lists: pgsql-hackers

Attachments

I wrote:
> As far as point 2 goes, I think this is another outgrowth of the
> fundamental design error that we load a partitioned rel's partitioning
> info immediately when the relcache entry is created, rather than later
> on-demand.  If we weren't doing that then it wouldn't be problematic
> to inspect the rel's rowtype while constructing the partitioning info.
> I've bitched about this before, if memory serves, but couldn't light
> a fire under anyone about fixing it.  Now I think we have no choice.
> It was never a great idea that minimal construction of a relcache
> entry could result in running arbitrary user-defined code.

Here's a draft patch for that.  There are a couple of secondary issues
I didn't do anything about yet:

* When rebuilding an open relcache entry for a partitioned table, this
coding now always quasi-leaks the old rd_pdcxt, where before that happened
only if the partdesc actually changed.  (Even if I'd kept the
equalPartitionDescs call, it would always fail.)  I complained about the
quasi-leak behavior before, but this probably pushes it up to the level of
"must fix".  What I'm inclined to do is to hack
RelationDecrementReferenceCount so that, when the refcount goes to zero,
we delete any child contexts of rd_pdcxt.  That's pretty annoying but in
the big scheme of things it's unlikely to matter.

* It'd be better to declare RelationGetPartitionKey and
RelationGetPartitionDesc in relcache.h and get their callers out of the
business of including rel.h, where possible.

* equalPartitionDescs is now dead code, should we remove it?

> Note that the end result of this would be to allow, not prohibit,
> cases like your example.  I wonder whether we couldn't also lift
> the restriction against whole-row Vars in partition expressions.
> Doesn't seem like there is much difference between such a Var and
> a row(...)::table_rowtype expression.

I didn't look into that either.  I wouldn't propose back-patching that,
but it'd be interesting to try to fix it in HEAD.

			regards, tom lane

Commits

  1. Fix CheckAttributeType's handling of collations for ranges.

  2. Improve comments in utils/rel.h.

  3. Allow whole-row Vars to be used in partitioning expressions.

  4. Remove equalPartitionDescs().

  5. Load relcache entries' partitioning data on-demand, not immediately.

  6. Disallow partition key expressions that return pseudo-types.

  7. Prevent a rowtype from being included in itself via a range.