Re: [HACKERS] Secondary index access optimizations

On 04.09.2017 12:59, Amit Langote wrote:
> Hi Konstantin,
>
> On 2017/09/04 18:19, Konstantin Knizhnik wrote:
>> On 04.09.2017 05:38, Amit Langote wrote:
>>> On 2017/09/02 12:44, Thomas Munro wrote:
>>>> On Wed, Aug 16, 2017 at 9:23 PM, Konstantin Knizhnik
>>>> <k.knizhnik@postgrespro.ru> wrote:
>>>>> postgres=# explain select * from bt where k between 1 and 20000 and v
>>>>> = 100;
>>>>>                                 QUERY PLAN
>>>>> ----------------------------------------------------------------------
>>>>>    Append  (cost=0.29..15.63 rows=2 width=8)
>>>>>      ->  Index Scan using dti1 on dt1  (cost=0.29..8.30 rows=1 width=8)
>>>>>            Index Cond: (v = 100)
>>>>>      ->  Index Scan using dti2 on dt2  (cost=0.29..7.33 rows=1 width=8)
>>>>>            Index Cond: (v = 100)
>>>>>            Filter: (k <= 20000)
>>>>> (6 rows)
>>>> +1
>>>>
>>>> This seems like a good feature to me: filtering stuff that is
>>>> obviously true is a waste of CPU cycles and may even require people to
>>>> add redundant stuff to indexes.  I was pondering something related to
>>>> this over in the partition-wise join thread (join quals that are
>>>> implied by partition constraints and should be discarded).
>>>>
>>>> It'd be interesting to get Amit Langote's feedback, so I CC'd him.
>>>> I'd be surprised if he and others haven't got a plan or a patch for
>>>> this down the back of the sofa.
>>> I agree that that's a good optimization in the cases it's correct.  Given
>>> that check_index_predicates() already applies the same optimization when
>>> considering using a partial index, it might make sense to try to do the
>>> same even earlier for the table itself using its CHECK / NOT NULL
>>> constraints as predicates (I said *earlier* because
>>> relation_excluded_by_constrains happens for a relation before we look at
>>> its indexes).  Also, at the end of relation_excluded_by_constraints() may
>>> not be such a bad place to do this.
>>>
>>> By the way, I read in check_index_predicates() that we should not apply
>>> this optimization if the relation in question is a target of UPDATE /
>>> DELETE / SELECT FOR UPDATE.
>> Please correct me if I wrong, but it seems to me that in case of table
>> constraints it is not necessary to specially handle update case.
>> As far as I understand we need to leave predicate in the plan in case of
>> partial indexes because due to "read committed" isolation policy
>> we may need to recheck that tuple still satisfies update condition (tuple
>> can be changed by some other committed transaction while we are waiting
>> for it and not satisfying this condition any more).
>> But no transaction can change tuple in such way that it violates table
>> constraints,  right? So we do not need to recheck it.
> Actually, I don't really know why check_index_predicates() skips this
> optimization in the target relation case, just wanted to point out that
> that's so.
>
> Thinking a bit from what you wrote, maybe we need not worry about
> EvalPlanQual in the context of your proposed optimization based on the
> table's constraints.
>
>> 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.

-- 
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company