Re: [HACKERS] Secondary index access optimizations - Mailing list pgsql-hackers

From Thomas Munro
Subject Re: [HACKERS] Secondary index access optimizations
Date
Msg-id CAEepm=0oS8drCnrBR9SsUBZFo8QQJAWaLVvcdPiqV9KOwfLLtQ@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Secondary index access optimizations  (Konstantin Knizhnik <k.knizhnik@postgrespro.ru>)
Responses Re: [HACKERS] Secondary index access optimizations
List pgsql-hackers
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.

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.

-- 
Thomas Munro
http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: Craig Ringer
Date:
Subject: Re: Redefining inet_net_ntop
Next
From: Tom Lane
Date:
Subject: Re: Failed test 'psql query died successfully after SIGQUIT'