Re: [HACKERS] Runtime Partition Pruning

[amul sul]

On Thu, Nov 9, 2017 at 4:48 PM, Beena Emerson  wrote:
> Hello all,
>
> Here is the updated patch which is rebased over v10 of Amit Langote's
> path towards faster pruning patch [1]. It modifies the PartScanKeyInfo
> struct to hold expressions which is then evaluated by the executor to
> fetch the correct partitions using the function.
>

Hi Beena,

I have started looking into your patch, here few initial comments
for your 0001 patch:

1.
 351 + * Evaluate and store the ooutput of ExecInitExpr for each
of the keys.

Typo: ooutput

2.
 822 +   if (IsA(constexpr, Const) &_runtime)
 823 +   continue;
 824 +
 825 +   if (IsA(constexpr, Param) &&!is_runtime)
 826 +   continue;
 827 +

 Add space after '&&'

 3.
 1095 +* Generally for appendrel we don't fetch the clause from the the

Typo: Double 'the'

4.
 272 
-/*-
 273 + 
/*-

 Unnecessary hunk.

5.
 313 +   Node   *n =
eval_const_expressions_from_list(estate->es_param_list_info, val);
 314 +

Crossing 80 column window.  Same at line # 323 & 325

6.
 315 +   keys->eqkeys_datums[i++] = ((Const *) n)->constvalue;

Don’t we need a check for IsA(n, Const) or assert ?

7.
1011 +   if (prmList)
1012 +   context.boundParams = prmList;  /* bound Params */
1013 +   else
1014 +   context.boundParams = NULL;

No need of prmList null check, context.boundParams = prmList; is enough.

8.  It would be nice if you create a separate patch where you are moving
PartScanKeyInfo and exporting function declaration.

9.  Could you please add few regression tests, that would help in
review & testing.

10. Could you please rebase your patch against latest "path toward faster
partition pruning" patch by Amit.

Regards,
Amul


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Runtime Partition Pruning

[Amit Kapila]

On Thu, Nov 9, 2017 at 9:01 PM, Robert Haas  wrote:
> On Thu, Nov 9, 2017 at 6:18 AM, Beena Emerson  wrote:
>> The code still chooses the custom plan instead of the generic plan for
>> the prepared statements. I am working on it.
>
> I don't think it's really the job of this patch to do anything about
> that problem.
>

+1.  I think if we really want to do something about plan choice when
partitions are involved that should be done as a separate patch.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Runtime Partition Pruning

[Robert Haas]

On Thu, Nov 9, 2017 at 6:18 AM, Beena Emerson  wrote:
> The code still chooses the custom plan instead of the generic plan for
> the prepared statements. I am working on it.

I don't think it's really the job of this patch to do anything about
that problem.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Runtime Partition Pruning

[Beena Emerson]

Hello all,

Here is the updated patch which is rebased over v10 of Amit Langote's
path towards faster pruning patch [1]. It modifies the PartScanKeyInfo
struct to hold expressions which is then evaluated by the executor to
fetch the correct partitions using the function.

The code still chooses the custom plan instead of the generic plan for
the prepared statements. I am working on it. The following output is
after adding a hack in the code forcing selection of generic plan.

postgres=# EXPLAIN EXECUTE prstmt_select(7);
QUERY PLAN
--
 Append  (cost=0.00..1732.25 rows=2 width=8)
   ->  Seq Scan on tprt_1  (cost=0.00..847.00 rows=16667 width=8)
 Filter: ($1 > col1)
   ->  Seq Scan on tprt_2  (cost=0.00..847.00 rows=16667 width=8)
 Filter: ($1 > col1)
(5 rows)

postgres=# EXPLAIN EXECUTE prstmt_select(20);
QUERY PLAN
--
 Append  (cost=0.00..1732.25 rows=3 width=8)
   ->  Seq Scan on tprt_1  (cost=0.00..847.00 rows=16667 width=8)
 Filter: ($1 > col1)
   ->  Seq Scan on tprt_2  (cost=0.00..847.00 rows=16667 width=8)
 Filter: ($1 > col1)
   ->  Seq Scan on tprt_3  (cost=0.00..38.25 rows=753 width=8)
 Filter: ($1 > col1)
(7 rows)


[1] 
https://www.postgresql.org/message-id/b8094e71-2c73-ed8e-d8c3-53f232c8c049%40lab.ntt.co.jp

Tested on commit: 9b9cb3c4534d717c1c95758670198ebbf8a20af2

-- 

Beena Emerson

EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


0001-Implement-runtime-partiton-pruning.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers