Re: crash with sql language partition support function

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: crash with sql language partition support function
Date: 2018-04-12 21:58:59
Message-ID: 20180412215859.mlvreahuv5xgvqfd@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wonder what prompted people to #include "catalog/partition.h" in
executor.h.

Amit Langote wrote:

> Anyway, after reading your replies, I thought of taking a stab at unifying
> the partitioning information that's cached by relcache.c.

After going over your patch, I think you went slightly overboard here.
Or maybe not, but this patch is so large that it's hard to form an
opinion about it. Some of these cleanups we should probably adopt per
discussion upthread, but I think we're at a point where we have to work
in smaller steps to avoid destabilizing the code.

I'm not sure about the new PartitionInfo that you propose. I see you
had to add partitioning/partbounds.h to rel.h -- not a fan of that.
I was thinking of a simpler fix there, just remove one of the two
memcxts in RelationData and put both structs in the remaining one.
Maybe I'm not ambitious enough.

Here's what I would propose: create partitioning/partbounds.c to deal
with partition bounds (i.e. mostly PartitionBoundInfoData and
PartitionBoundSpec), and have utils/cache/partcache.c (with
corresponding utils/partcache.h) for RelationGetPartitionDesc and
RelationGetPartitionKey (not much else, it seems).

Maybe also move get_partition_for_tuple to execPartition.c?

Preliminarly, I would say that the following functions would be in
partbounds.c (in roughly this order):

Exported:

get_qual_from_partbound
partition_bounds_equal
partition_bounds_copy
check_new_partition_bound
check_default_allows_bound
get_hash_partition_greatest_modulus
make_one_range_bound

qsort_partition_list_value_cmp
partition_rbound_cmp
partition_rbound_datum_cmp
qsort_partition_hbound_cmp
partition_hbound_cmp

partition_list_bsearch
partition_range_bsearch
partition_range_datum_bsearch
partition_hash_bsearch

static:
get_partition_bound_num_indexes
make_partition_op_expr
get_partition_operator
get_qual_for_hash
get_qual_for_list
get_qual_for_range
get_range_key_properties
get_range_nulltest

Unsure yet about compute_hash_value and satisfies_hash_partition.

The rest would remain in catalog/partition.c, which should hopefully not
be a lot.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-04-12 22:01:52 Re: [HACKERS] Runtime Partition Pruning
Previous Message Tom Lane 2018-04-12 21:10:15 Re: psql leaks memory on query cancellation