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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers