On Thu, Nov 9, 2017 at 4:48 PM, Beena Emerson <memissemer...@gmail.com> 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) &&is_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
