pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: David Rowley <david.rowley@2ndquadrant.com>
To: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>
Cc: Beena Emerson <memissemerson@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: amul sul <sulamul@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Subject: Re: [HACKERS] Runtime Partition Pruning
Date: Thu, 22 Feb 2018 18:31:23 +0900
Message-ID: <31e27e19-eaf6-642f-8eb6-9cc7528fa57b@lab.ntt.co.jp> (raw)
In-Reply-To: <CAKJS1f_bzRdbUutwc8iy5Bs5Krc9couEQK9NH1SUgMJvvnvWxg@mail.gmail.com>
References: <CAKJS1f-zohKp_ANHYMe49nNje4vb81nF9s9EicK8KsjqQNkQ1A@mail.gmail.com>
	<20180104163708.bzgheqsny6tsatzf@alvherre.pgsql>
	<CAKJS1f9Tavq5MuVVuzjp9EvPtj8GBnyunBUX61sERc9dwjWVMQ@mail.gmail.com>
	<CAOG9ApGqEOrdMefche6uOM4fkUCz0uE9LMP6JHiSJEQ8CySdaQ@mail.gmail.com>
	<CAKJS1f8gPmRkJkHQ6WbKYS+nDxXT2fKDjcT_xED2k37eWW0pYw@mail.gmail.com>
	<CAKJS1f96m-VMW-ViAXOdV-T8h_KRdrCUmADYNrhh2hySPz5Y=Q@mail.gmail.com>
	<CAOG9ApEuPL02ZCpAwxDBgkaqs1K-i4ZfVkTrpc_x8B_Eq5OtGA@mail.gmail.com>
	<CAOG9ApERiop7P=GRkqQKa82AuBKjxN3qVixie3WK4WqQpEjS6g@mail.gmail.com>
	<CAKJS1f8iop-r82tm6-1wrWwqiphpMzuSsCSFg-Ndns+Qg3ukPg@mail.gmail.com>
	<CAKJS1f_vSFZFSOW6Dspj87iU9BXvbAiVesviDZMCfYchVP+mhA@mail.gmail.com>
	<CAKcux6mf146esdNqa6Cv5RegiqGTNecN6mpoXMqnsCT7Pg5YxQ@mail.gmail.com>
	<CAKJS1f8rF+7WOu95WHH_pFu6NFh0iyF3oi+JZKGwaZbDXnJQ3A@mail.gmail.com>
	<CAKJS1f_bzRdbUutwc8iy5Bs5Krc9couEQK9NH1SUgMJvvnvWxg@mail.gmail.com>

Hi David.

On 2018/02/21 18:06, David Rowley wrote:
> Other things I don't particularly like about the current patch are how
> I had to construct the partition key expressions in
> set_valid_runtime_subplans_recurse(). This pretty much uses code
> borrowed from set_baserel_partition_key_exprs(). One way around that
> would be to change the partprune.c code to deal with the
> partkey->partattrs and consume an expression from the list on attnum =
> 0. I didn't do that as I want to minimise how much I touch Amit's
> patch before it gets committed as doing so would likely just cause
> more headaches for me keeping this merged with his work. Another
> option to resolve this would be to put the partition key expressions
> into the PartitionPruneInfo.

Another way could be to refactor the code you've borrowed from
set_baserel_partition_key_exprs() into its own function that is exported
by some optimizer header.

I removed PartitionKey/Relation from signatures of various functions of my
patch to avoid having to re-heap_open() the relation per [1].

> I've attached v11 of the patch.

Some comments:

* I noticed that the patch adds a function to bitmapset.c which you might
want to extract into its own patch, like your patch to add bms_add_range()
that got committed as 84940644d [2].

* Maybe it's better to add `default: break;` in the two switch's
you've added in partkey_datum_from_expr()

partprune.c: In function ‘partkey_datum_from_expr’:
partprune.c:1555:2: warning: enumeration value ‘T_Invalid’ not handled in
switch [-Wswitch]
  switch (nodeTag(expr))

partprune.c:1562:4: warning: enumeration value ‘PARAM_SUBLINK’ not handled
in switch [-Wswitch]
    switch (((Param *) expr)->paramkind)

* I see that you moved PartitionClauseInfo's definition from partprune.c
to partition.h.  Isn't it better to move it to partprune.h?

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CA%2BTgmoabi-29Vs8H0xkjtYB%3DcU%2BGVCrNwPz7okpa3KsoLmdEUQ%40ma...

[2] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=84940644d





view thread (186+ messages)  latest in thread

Message-ID: <31e27e19-eaf6-642f-8eb6-9cc7528fa57b@lab.ntt.co.jp>
Permalink:  ../31e27e19-eaf6-642f-8eb6-9cc7528fa57b@lab.ntt.co.jp/
Also on:    postgresql.org/message-id/31e27e19-eaf6-642f-8eb6-9cc7528fa57b@lab.ntt.co.jp

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: Langote_Amit_f8@lab.ntt.co.jp, david.rowley@2ndquadrant.com, rajkumar.raghuwanshi@enterprisedb.com, memissemerson@gmail.com, alvherre@alvh.no-ip.org, robertmhaas@gmail.com, sulamul@gmail.com, dilipbalaut@gmail.com
  Subject: Re: [HACKERS] Runtime Partition Pruning
  In-Reply-To: <31e27e19-eaf6-642f-8eb6-9cc7528fa57b@lab.ntt.co.jp>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox