Re: [HACKERS] Secondary index access optimizations

On 29.01.2018 07:34, Thomas Munro wrote:
> On Sat, Jan 20, 2018 at 5:41 AM, Konstantin Knizhnik
> <k.knizhnik@postgrespro.ru> wrote:
>> On 19.01.2018 16:14, Antonin Houska wrote:
>>> you should test the operator B-tree strategy: BTLessStrategyNumber,
>>> BTLessEqualStrategyNumber, etc. The operator string alone does not tell
>>> enough
>>> about the operator semantics.
>>>
>>> The strategy can be found in the pg_amop catalog.
>>> get_op_btree_interpretation() function may be useful, but there may be
>>> something better in utils/cache/lsyscache.c.
>>>
>> Thank you very much.
>> Shame on me that I didn't notice such solution myself - such checking of
>> B-tree strategy is done in the same predtest.c file!
>> Now checking of predicate clauses compatibility is done in much more elegant
>> and general way.
>> Attached please find new version of the patch.
> I wonder if you should create a new index and SysCache entry for
> looking up range types by subtype.  I'll be interested to see what
> others have to say about this range type-based technique -- it seems
> clever to me, but I'm not familiar enough with this stuff to say if
> there is some other approach you should be using instead.
I think that it is good idea to add caching for range type lookup.
If community think that it will be useful I can try to add such mechanism.
But it seems to be not so trivial, especially properly handle invalidations.

> Some superficial project style comments (maybe these could be fixed
> automatically with pgindent?):
>
> +static TypeCacheEntry* lookup_range_type(Oid type)
>
> +static RangeType* operator_to_range(TypeCacheEntry *typcache, Oid
> oper, Const* literal)
>
> ... these should be like this:
>
> static RangeType *
> operator_to_range(...
>
> I think the idea is that you can always search for a function
> definition with by looking for "name(" at the start of a line, so we
> put a newline there.  Then there is the whitespace before "*", not
> after it.
>
> + if (pred_range && clause_range && range_eq_internal(typcache,
> pred_range, clause_range))
> + {
> + return true;
> + }
>
> Unnecessary braces.
>
> +/*
> + * Get range type for the corresprent scalar type.
> + * Returns NULl if such range type is not found.
> + * This function performs sequential scan in pg_range table,
> + * but since number of range rtype is not expected to be large (6
> builtin range types),
> + * it should not be a problem.
> + */
>
> Typos.
>
Thank you.
I fixed this issues.
New patch is attached.

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