On 2017/09/04 20:46, Konstantin Knizhnik wrote:
> On 04.09.2017 12:59, Amit Langote wrote:
>> On 2017/09/04 18:19, Konstantin Knizhnik wrote:
>>> Concerning your suggestion to merge check_index_predicates() and
>>> remove_restrictions_implied_by_constraints() functions: may be it can be
>>> done, but frankly speaking I do not see much sense in it - there are too
>>> much differences between this functions and too few code reusing.
>> Maybe, you meant to address Thomas here. :) Reading his comment again, I
>> too am a bit concerned about destructively modifying the input rel's
>> baserestrictinfo. There should at least be a comment that that's being
>> done.
>
> But I have considered Thomas comment and extracted code updating
> relation's baserestrictinfo from
> relation_excluded_by_constraints() to
> remove_restrictions_implied_by_constraints() function. It was included in
> new version of the patch.
Sorry, I hadn't noticed the new patch.
I was confused because I didn't suggest "to merge check_index_predicates()
and remove_restrictions_implied_by_constraints() functions". Perhaps, the
wording in my previous message wasn't clear.
Looking at the new patch, the changes regarding
remove_restrictions_implied_by_constraints() look fine.
Like Thomas, I'm not so sure about the whole predtest.c patch. The core
logic in operator_predicate_proof() should be able to conclude that, say,
k < 21 is implied by k <= 20, which you are trying to address with some
special case code. If there is still problem you think need to be fixed
here, a better place to look at would be somewhere around get_btree_test_op().
Thanks,
Amit
