On Sat, Nov 8, 2014 at 4:16 AM, Robert Haas <robertmh...@gmail.com> wrote: > > On Mon, Oct 27, 2014 at 2:35 AM, Kouhei Kaigai <kai...@ak.jp.nec.com> wrote: > >> FYI, patch v12 part 2 no longer applies cleanly. > >> > > Thanks. I rebased the patch set according to the latest master branch. > > The attached v13 can be applied to the master. > > I've committed parts 1 and 2 of this, without the documentation, and > with some additional cleanup.
Few observations/questions related to this commit: 1. @@ -5546,6 +5568,29 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) colinfo = deparse_columns_fetch(var->varno, dpns); attnum = var->varattno; } + else if (IS_SPECIAL_VARNO(var->varno) && + IsA(dpns->planstate, CustomScanState) && + (expr = GetSpecialCustomVar((CustomScanState *) dpns->planstate, + var, &child_ps)) != NULL) + { + deparse_namespace save_dpns; + + if (child_ps) + push_child_plan(dpns, child_ps, &save_dpns); + /* + * Force parentheses because our caller probably assumed a Var is a + * simple expression. + */ + if (!IsA(expr, Var)) + appendStringInfoChar(buf, '('); + get_rule_expr((Node *) expr, context, true); + if (!IsA(expr, Var)) + appendStringInfoChar(buf, ')'); + + if (child_ps) + pop_child_plan(dpns, &save_dpns); + return NULL; + } a. It seems Assert for netlelvelsup is missing in this loop. b. Below comment in function get_variable can be improved w.r.t handling for CustomScanState. The comment indicates that if varno is OUTER_VAR or INNER_VAR or INDEX_VAR, it handles all of them similarly which seems to be slightly changed for CustomScanState. /* * Try to find the relevant RTE in this rtable. In a plan tree, it's * likely that varno is OUTER_VAR or INNER_VAR, in which case we must dig * down into the subplans, or INDEX_VAR, which is resolved similarly. Also * find the aliases previously assigned for this RTE. */ 2. +void +register_custom_path_provider(CustomPathMethods *cpp_methods) { .. } Shouldn't there be unregister function corresponding to above register function? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com