Re: Declarative partitioning

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.

>> 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.

Am I missing something?

Thanks,
Amit