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