Re: Runtime Partition Pruning

On Thu, Nov 9, 2017 at 4:48 PM, Beena Emerson <memissemerson(at)gmail(dot)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