On Mon, Apr 18, 2016 at 1:23 PM, Amit Langote <langote_amit...@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
