Re: Declarative partitioning - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: Declarative partitioning
Date
Msg-id CAFjFpRf2HrvAGUr8VCc7HL1ZhM8ub0dUKcZFyPnw_=p+sD3RAA@mail.gmail.com
Whole thread Raw
In response to Re: Declarative partitioning  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
List pgsql-hackers


On Mon, Apr 18, 2016 at 1:23 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
On 2016/04/18 15:38, Ashutosh Bapat wrote:
>> There was no KeyTypeCollInfo in early days of the patch and then I found
>> myself doing a lot of:
>>
>> partexprs_item = list_head(key->partexprs);
>> for (attr in key->partattrs)
>> {
>>     if (attr->attnum != 0)
>>     {
>>         // simple column reference, get type from attr
>>     }
>>     else
>>     {
>>         // expression, get type using exprType, etc.
>>         partexprs_item = lnext(partexprs_item);
>>     }
>> }
>>
>
> At least the two loops can be flattened to a single loop if we keep only
> expressions list with attributes being just Var nodes. exprType() etc.
> would then work seemlessly.

I didn't say anything about your suggestion to use a Node * list as a
representation for the cached partition key information.  IIUC, you mean
instead of the AttrNumber[partnatts] array with non-zero attnum for a
named column slot and 0 for a expressional column slot, create a Node *
list with Var nodes for simple column references and Expr nodes for
expressions.

I would mention that the same information is also being used in contexts
where having simple attnums may be better (for example, when extracting
key of a tuple slot during tuple routing).  Moreover, this is cached
information and I thought it may be better to follow the format that other
similar information uses (index key and such).  Furthermore, looking at
qual matching code for indexes and recently introduced foreign key
optimization, it seems we will want to use a similar representation within
optimizer for partition keys.  IndexOptInfo has int ncolumns and int *
indexkeys and then match_index_to_operand() compares index key attnums
with varattno of vars in qual.  It's perhaps speculative at the moment
because there is not much code wanting to use it yet other than partition
DDL and tuple-routing and  cached info seems to work as-is for the latter.

Ok.
 

>> That ended up being quite a few places (though I managed to reduce the
>> number of places over time).  So, I created this struct which is
>> initialized when partition key is built (on first open of the partitioned
>> table).
>>
>
> Hmm, I am just afraid that we might end up with some code using cached
> information and some using exprType, exprTypmod etc.

Well, you never use exprType(), etc. for partition keys in other than a
few places.  All places that do always use the cached values.  Mostly
partitioning DDL stuff so far.  Tuple routing considers collation of
individual key columns when comparing input value with partition bounds.


I am not worried about the current code. But there will be a lot of code added after version 1. I am worried about that.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

pgsql-hackers by date:

Previous
From: Amit Langote
Date:
Subject: Description of ForeignPath
Next
From: Robins Tharakan
Date:
Subject: Re: Pgbench with -f and -S