On Mon, May 8, 2017 at 2:59 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Yes, disallowing this in the first place is the best thing to do.
> Attached patch 0001 implements that. With the patch:
Committed.
With regard to 0002, some of the resulting constraints are a bit more
complicated than seems desirable:
create table foo1 partition of foo for values from (unbounded,
unbounded, unbounded) to (1, unbounded, unbounded);
yields:
Partition constraint: CHECK (((a < 1) OR (a = 1) OR (a = 1)))
It would be better not to have (a = 1) in there twice, and better
still to have the whole thing as (a <= 1).
create table foo2 partition of foo for values from (3, 4, 5) to (6, 7,
unbounded);
yields:
Partition constraint: CHECK ((((a > 3) OR ((a = 3) AND (b > 4)) OR ((a
= 3) AND (b = 4) AND (c >= 5))) AND ((a < 6) OR ((a = 6) AND (b < 7))
OR ((a = 6) AND (b = 7)))))
The first half of that (for the lower bound) is of course fine, but
the second half could be written better using <=, like instead of
((a = 6) AND (b < 7)) OR ((a = 6) AND (b = 7))
you could have:
((a = 6) AND (b <= 7)
This isn't purely cosmetic because the simpler constraint is probably
noticeably faster to evaluate.
I think you should have a few test cases like this in the patch - that
is, cases where some but not all bounds are finite.
> BTW, is it strange that the newly added pg_get_partition_constraintdef()
> requires the relcache entry to be created for the partition and all of its
> ancestor relations up to the root (I mean the fact that the relcache entry
> needs to be created at all)? I can see only one other function,
> set_relation_column_names(), creating the relcache entry at all.
I suggest that you display this information only when "verbose" is set
- i.e. \d+ not just \d. I don't intrinsically care think that forcing
the relcache entry to be built here, but note that it means this will
block when the parent is locked. Between that and the fact that this
information will only sometimes be of interest, restricting it to \d+
seems preferable.
Next update on this issue by Thursday 5/11.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
