From 1c560dfa9c5cb6338277f4c5b1362145e5e2c10b Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Thu, 29 Sep 2016 15:37:40 -0600 Subject: [PATCH] Update ruleutils_95 with latest PostgreSQL changes Hand-applied changes from a diff I generated between 9.5.0 and 9.5.4. --- src/backend/distributed/utils/ruleutils_95.c | 116 ++++++++++++++----- 1 file changed, 84 insertions(+), 32 deletions(-) diff --git a/src/backend/distributed/utils/ruleutils_95.c b/src/backend/distributed/utils/ruleutils_95.c index 616e62c8f..2b710aa93 100644 --- a/src/backend/distributed/utils/ruleutils_95.c +++ b/src/backend/distributed/utils/ruleutils_95.c @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------- * - * citus_ruleutils.c + * ruleutils_95.c * Additional, non core exposed, functions to convert stored * expressions/querytrees back to source text * @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * src/backend/distributed/utils/citus_ruleutils.c + * src/backend/distributed/utils/ruleutils_95.c * * This needs to be closely in sync with the core code. *------------------------------------------------------------------------- @@ -3062,25 +3062,24 @@ get_insert_query_def(Query *query, deparse_context *context) /* Add a WHERE clause (for partial indexes) if given */ if (confl->arbiterWhere != NULL) { - bool varprefixInitialValue = context->varprefix; + bool save_varprefix; + + /* + * Force non-prefixing of Vars, since parser assumes that they + * belong to target relation. WHERE clause does not use + * InferenceElem, so this is separately required. + */ + save_varprefix = context->varprefix; + context->varprefix = false; appendContextKeyword(context, " WHERE ", -PRETTYINDENT_STD, PRETTYINDENT_STD, 1); - - /* - * Postgres deparses arbiter WHERE clauses incorrectly. It adds - * varprefix to the arbiter WHERE clause, for which Postgres parser - * errors out. Thus, we temporarily set varprefix to false. - */ - context->varprefix = false; - get_rule_expr(confl->arbiterWhere, context, false); - /* now set the varprefix back to its initial value */ - context->varprefix = varprefixInitialValue; + context->varprefix = save_varprefix; } } - else if (confl->constraint != InvalidOid) + else if (OidIsValid(confl->constraint)) { char *constraint = get_constraint_name(confl->constraint); int64 shardId = context->shardid; @@ -3090,8 +3089,11 @@ get_insert_query_def(Query *query, deparse_context *context) AppendShardIdToName(&constraint, shardId); } + if (!constraint) + elog(ERROR, "cache lookup failed for constraint %u", + confl->constraint); appendStringInfo(buf, " ON CONSTRAINT %s", - quote_qualified_identifier(NULL, constraint)); + quote_identifier(constraint)); } if (confl->action == ONCONFLICT_NOTHING) @@ -3592,7 +3594,8 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) tle = get_tle_by_resno(dpns->inner_tlist, var->varattno); if (!tle) - elog(ERROR, "bogus varattno for subquery var: %d", var->varattno); + elog(ERROR, "invalid attnum %d for relation \"%s\"", + var->varattno, rte->eref->aliasname); Assert(netlevelsup == 0); push_child_plan(dpns, dpns->inner_planstate, &save_dpns); @@ -3653,9 +3656,13 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) else if (attnum > 0) { /* Get column name to use from the colinfo struct */ - Assert(attnum <= colinfo->num_cols); + if (attnum > colinfo->num_cols) + elog(ERROR, "invalid attnum %d for relation \"%s\"", + attnum, rte->eref->aliasname); attname = colinfo->colnames[attnum - 1]; - Assert(attname != NULL); + if (attname == NULL) /* dropped column? */ + elog(ERROR, "invalid attnum %d for relation \"%s\"", + attnum, rte->eref->aliasname); } else if (GetRangeTblKind(rte) == CITUS_RTE_SHARD) { @@ -4791,6 +4798,24 @@ get_rule_expr(Node *node, deparse_context *context, get_base_element_type(exprType(arg2))), expr->useOr ? "ANY" : "ALL"); get_rule_expr_paren(arg2, context, true, node); + + /* + * There's inherent ambiguity in "x op ANY/ALL (y)" when y is + * a bare sub-SELECT. Since we're here, the sub-SELECT must + * be meant as a scalar sub-SELECT yielding an array value to + * be used in ScalarArrayOpExpr; but the grammar will + * preferentially interpret such a construct as an ANY/ALL + * SubLink. To prevent misparsing the output that way, insert + * a dummy coercion (which will be stripped by parse analysis, + * so no inefficiency is added in dump and reload). This is + * indeed most likely what the user wrote to get the construct + * accepted in the first place. + */ + if (IsA(arg2, SubLink) && + ((SubLink *) arg2)->subLinkType == EXPR_SUBLINK) + appendStringInfo(buf, "::%s", + format_type_with_typemod(exprType(arg2), + exprTypmod(arg2))); appendStringInfoChar(buf, ')'); if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, ')'); @@ -5452,17 +5477,43 @@ get_rule_expr(Node *node, deparse_context *context, if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, '('); get_rule_expr_paren((Node *) ntest->arg, context, true, node); - switch (ntest->nulltesttype) + + /* + * For scalar inputs, we prefer to print as IS [NOT] NULL, + * which is shorter and traditional. If it's a rowtype input + * but we're applying a scalar test, must print IS [NOT] + * DISTINCT FROM NULL to be semantically correct. + */ + if (ntest->argisrow || + !type_is_rowtype(exprType((Node *) ntest->arg))) { - case IS_NULL: - appendStringInfoString(buf, " IS NULL"); - break; - case IS_NOT_NULL: - appendStringInfoString(buf, " IS NOT NULL"); - break; - default: - elog(ERROR, "unrecognized nulltesttype: %d", - (int) ntest->nulltesttype); + switch (ntest->nulltesttype) + { + case IS_NULL: + appendStringInfoString(buf, " IS NULL"); + break; + case IS_NOT_NULL: + appendStringInfoString(buf, " IS NOT NULL"); + break; + default: + elog(ERROR, "unrecognized nulltesttype: %d", + (int) ntest->nulltesttype); + } + } + else + { + switch (ntest->nulltesttype) + { + case IS_NULL: + appendStringInfoString(buf, " IS NOT DISTINCT FROM NULL"); + break; + case IS_NOT_NULL: + appendStringInfoString(buf, " IS DISTINCT FROM NULL"); + break; + default: + elog(ERROR, "unrecognized nulltesttype: %d", + (int) ntest->nulltesttype); + } } if (!PRETTY_PAREN(context)) appendStringInfoChar(buf, ')'); @@ -5550,13 +5601,14 @@ get_rule_expr(Node *node, deparse_context *context, case T_InferenceElem: { InferenceElem *iexpr = (InferenceElem *) node; - bool varprefix = context->varprefix; + bool save_varprefix; bool need_parens; /* * InferenceElem can only refer to target relation, so a - * prefix is never useful. + * prefix is not useful, and indeed would cause parse errors. */ + save_varprefix = context->varprefix; context->varprefix = false; /* @@ -5576,7 +5628,7 @@ get_rule_expr(Node *node, deparse_context *context, if (need_parens) appendStringInfoChar(buf, ')'); - context->varprefix = varprefix; + context->varprefix = save_varprefix; if (iexpr->infercollid) appendStringInfo(buf, " COLLATE %s", @@ -7331,4 +7383,4 @@ generate_operator_name(Oid operid, Oid arg1, Oid arg2) return buf.data; } -#endif +#endif /* (PG_VERSION_NUM >= 90500 && PG_VERSION_NUM < 90600) */