Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjusts query's CoerceViaIO & RelabelType nodes that are improper for deparsing #6391

Merged
merged 2 commits into from
May 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 9 additions & 31 deletions src/backend/distributed/deparser/ruleutils_13.c
Original file line number Diff line number Diff line change
Expand Up @@ -5246,42 +5246,20 @@ get_rule_expr(Node *node, deparse_context *context,
case T_RelabelType:
{
RelabelType *relabel = (RelabelType *) node;
Node *arg = (Node *) relabel->arg;

/*
* This is a Citus specific modification
* The planner converts CollateExpr to RelabelType
* and here we convert back.
*/
if (relabel->resultcollid != InvalidOid)
if (relabel->relabelformat == COERCE_IMPLICIT_CAST &&
!showimplicit)
{
CollateExpr *collate = RelabelTypeToCollateExpr(relabel);
Node *arg = (Node *) collate->arg;

if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, '(');
get_rule_expr_paren(arg, context, showimplicit, node);
appendStringInfo(buf, " COLLATE %s",
generate_collation_name(collate->collOid));
if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, ')');
/* don't show the implicit cast */
get_rule_expr_paren(arg, context, false, node);
}
else
{
Node *arg = (Node *) relabel->arg;

if (relabel->relabelformat == COERCE_IMPLICIT_CAST &&
!showimplicit)
{
/* don't show the implicit cast */
get_rule_expr_paren(arg, context, false, node);
}
else
{
get_coercion_expr(arg, context,
relabel->resulttype,
relabel->resulttypmod,
node);
}
get_coercion_expr(arg, context,
relabel->resulttype,
relabel->resulttypmod,
node);
}
}
break;
Expand Down
40 changes: 9 additions & 31 deletions src/backend/distributed/deparser/ruleutils_14.c
Original file line number Diff line number Diff line change
Expand Up @@ -5470,42 +5470,20 @@ get_rule_expr(Node *node, deparse_context *context,
case T_RelabelType:
{
RelabelType *relabel = (RelabelType *) node;
Node *arg = (Node *) relabel->arg;

/*
* This is a Citus specific modification
* The planner converts CollateExpr to RelabelType
* and here we convert back.
*/
if (relabel->resultcollid != InvalidOid)
if (relabel->relabelformat == COERCE_IMPLICIT_CAST &&
!showimplicit)
{
CollateExpr *collate = RelabelTypeToCollateExpr(relabel);
Node *arg = (Node *) collate->arg;

if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, '(');
get_rule_expr_paren(arg, context, showimplicit, node);
appendStringInfo(buf, " COLLATE %s",
generate_collation_name(collate->collOid));
if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, ')');
/* don't show the implicit cast */
get_rule_expr_paren(arg, context, false, node);
}
else
{
Node *arg = (Node *) relabel->arg;

if (relabel->relabelformat == COERCE_IMPLICIT_CAST &&
!showimplicit)
{
/* don't show the implicit cast */
get_rule_expr_paren(arg, context, false, node);
}
else
{
get_coercion_expr(arg, context,
relabel->resulttype,
relabel->resulttypmod,
node);
}
get_coercion_expr(arg, context,
relabel->resulttype,
relabel->resulttypmod,
node);
}
}
break;
Expand Down
40 changes: 9 additions & 31 deletions src/backend/distributed/deparser/ruleutils_15.c
Original file line number Diff line number Diff line change
Expand Up @@ -5696,42 +5696,20 @@ get_rule_expr(Node *node, deparse_context *context,
case T_RelabelType:
{
RelabelType *relabel = (RelabelType *) node;
Node *arg = (Node *) relabel->arg;

/*
* This is a Citus specific modification
* The planner converts CollateExpr to RelabelType
* and here we convert back.
*/
if (relabel->resultcollid != InvalidOid)
if (relabel->relabelformat == COERCE_IMPLICIT_CAST &&
!showimplicit)
{
CollateExpr *collate = RelabelTypeToCollateExpr(relabel);
Node *arg = (Node *) collate->arg;

if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, '(');
get_rule_expr_paren(arg, context, showimplicit, node);
appendStringInfo(buf, " COLLATE %s",
generate_collation_name(collate->collOid));
if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, ')');
/* don't show the implicit cast */
get_rule_expr_paren(arg, context, false, node);
}
else
{
Node *arg = (Node *) relabel->arg;

if (relabel->relabelformat == COERCE_IMPLICIT_CAST &&
!showimplicit)
{
/* don't show the implicit cast */
get_rule_expr_paren(arg, context, false, node);
}
else
{
get_coercion_expr(arg, context,
relabel->resulttype,
relabel->resulttypmod,
node);
}
get_coercion_expr(arg, context,
relabel->resulttype,
relabel->resulttypmod,
node);
}
}
break;
Expand Down
145 changes: 144 additions & 1 deletion src/backend/distributed/planner/multi_physical_planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "access/xlog.h"
#include "catalog/pg_aggregate.h"
#include "catalog/pg_am.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
Expand Down Expand Up @@ -69,6 +70,7 @@
#include "optimizer/restrictinfo.h"
#include "optimizer/tlist.h"
#include "parser/parse_relation.h"
#include "parser/parse_type.h"
#include "parser/parsetree.h"
#include "rewrite/rewriteManip.h"
#include "utils/builtins.h"
Expand All @@ -79,6 +81,7 @@
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/syscache.h"
#include "utils/typcache.h"

/* RepartitionJoinBucketCountPerNode determines bucket amount during repartitions */
Expand Down Expand Up @@ -231,6 +234,11 @@ static List * FetchEqualityAttrNumsForRTEBoolExpr(BoolExpr *boolExpr);
static List * FetchEqualityAttrNumsForList(List *nodeList);
static int PartitionColumnIndex(Var *targetVar, List *targetList);
static List * GetColumnOriginalIndexes(Oid relationId);
static bool QueryTreeHasImproperForDeparseNodes(Node *inputNode);
static Node * AdjustImproperForDeparseNodes(Node *inputNode);
static bool IsImproperForDeparseRelabelTypeNode(Node *inputNode);
static bool IsImproperForDeparseCoerceViaIONode(Node *inputNode);
static CollateExpr * RelabelTypeToCollateExpr(RelabelType *relabelType);


/*
Expand Down Expand Up @@ -2683,6 +2691,18 @@ SqlTaskList(Job *job)
List *fragmentCombinationList = FragmentCombinationList(rangeTableFragmentsList,
jobQuery, dependentJobList);

/*
* Adjust RelabelType and CoerceViaIO nodes that are improper for deparsing.
* We first check if there are any such nodes by using a query tree walker.
* The reason is that a query tree mutator will create a deep copy of all
* the query sublinks, and we don't want to do that unless necessary, as it
* would be inefficient.
*/
if (QueryTreeHasImproperForDeparseNodes((Node *) jobQuery))
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to have a comment on why we first check the query tree and then only do AdjustImproperForDeparseNodes later. It might not be obvious for the readers.

jobQuery = (Query *) AdjustImproperForDeparseNodes((Node *) jobQuery);
}

ListCell *fragmentCombinationCell = NULL;
foreach(fragmentCombinationCell, fragmentCombinationList)
{
Expand Down Expand Up @@ -2733,7 +2753,7 @@ SqlTaskList(Job *job)
* RelabelTypeToCollateExpr converts RelabelType's into CollationExpr's.
* With that, we will be able to pushdown COLLATE's.
*/
CollateExpr *
static CollateExpr *
RelabelTypeToCollateExpr(RelabelType *relabelType)
{
Assert(OidIsValid(relabelType->resultcollid));
Expand Down Expand Up @@ -5593,3 +5613,126 @@ TaskListHighestTaskId(List *taskList)

return highestTaskId;
}


/*
* QueryTreeHasImproperForDeparseNodes walks over the node,
* and returns true if there are RelabelType or
* CoerceViaIONodes which are improper for deparse
*/
static bool
QueryTreeHasImproperForDeparseNodes(Node *inputNode)
{
if (inputNode == NULL)
{
return false;
}
else if (IsImproperForDeparseRelabelTypeNode(inputNode) ||
IsImproperForDeparseCoerceViaIONode(inputNode))
{
return true;
}
else if (IsA(inputNode, Query))
{
return query_tree_walker((Query *) inputNode,
QueryTreeHasImproperForDeparseNodes,
NULL, 0);
}

return expression_tree_walker(inputNode,
QueryTreeHasImproperForDeparseNodes,
NULL);
}


/*
* AdjustImproperForDeparseNodes takes an input rewritten query and modifies
* nodes which, after going through our planner, pose a problem when
* deparsing. So far we have two such type of Nodes that may pose problems:
* RelabelType and CoerceIO nodes.
* Details will be written in comments in the corresponding if conditions.
*/
static Node *
AdjustImproperForDeparseNodes(Node *inputNode)
{
if (inputNode == NULL)
{
return NULL;
}

if (IsImproperForDeparseRelabelTypeNode(inputNode))
{
/*
* The planner converts CollateExpr to RelabelType
* and here we convert back.
*/
return (Node *) RelabelTypeToCollateExpr((RelabelType *) inputNode);
}
else if (IsImproperForDeparseCoerceViaIONode(inputNode))
{
/*
* The planner converts some ::text/::varchar casts to ::cstring
* and here we convert back to text because cstring is a pseudotype
* and it cannot be casted to most resulttypes
*/

CoerceViaIO *iocoerce = (CoerceViaIO *) inputNode;
Node *arg = (Node *) iocoerce->arg;
Const *cstringToText = (Const *) arg;

cstringToText->consttype = TEXTOID;
cstringToText->constlen = -1;

Type textType = typeidType(TEXTOID);
char *constvalue = NULL;

if (!cstringToText->constisnull)
{
constvalue = DatumGetCString(cstringToText->constvalue);
}

cstringToText->constvalue = stringTypeDatum(textType,
constvalue,
cstringToText->consttypmod);
ReleaseSysCache(textType);
return inputNode;
}
else if (IsA(inputNode, Query))
{
return (Node *) query_tree_mutator((Query *) inputNode,
AdjustImproperForDeparseNodes,
NULL, QTW_DONT_COPY_QUERY);
}

return expression_tree_mutator(inputNode, AdjustImproperForDeparseNodes, NULL);
}


/*
* Checks if the given node is of Relabel type which is improper for deparsing
* The planner converts some CollateExpr to RelabelType nodes, and we need
* to find these nodes. They would be improperly deparsed without the
* "COLLATE" expression.
*/
static bool
IsImproperForDeparseRelabelTypeNode(Node *inputNode)
{
return (IsA(inputNode, RelabelType) &&
OidIsValid(((RelabelType *) inputNode)->resultcollid) &&
((RelabelType *) inputNode)->resultcollid != DEFAULT_COLLATION_OID);
}


/*
* Checks if the given node is of CoerceViaIO type which is improper for deparsing
* The planner converts some ::text/::varchar casts to ::cstring, and we need
* to find these nodes. They would be improperly deparsed with "cstring" which cannot
* be casted to most resulttypes.
*/
static bool
IsImproperForDeparseCoerceViaIONode(Node *inputNode)
{
return (IsA(inputNode, CoerceViaIO) &&
IsA(((CoerceViaIO *) inputNode)->arg, Const) &&
((Const *) ((CoerceViaIO *) inputNode)->arg)->consttype == CSTRINGOID);
}
1 change: 0 additions & 1 deletion src/include/distributed/multi_physical_planner.h
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,6 @@ extern Node * WrapUngroupedVarsInAnyValueAggregate(Node *expression,
List *groupClauseList,
List *targetList,
bool checkExpressionEquality);
extern CollateExpr * RelabelTypeToCollateExpr(RelabelType *relabelType);

/*
* Function declarations for building, updating constraints and simple operator
Expand Down
4 changes: 2 additions & 2 deletions src/test/regress/expected/distributed_collations.out
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ NOTICE: renaming the new table to collation_tests.test_collate_pushed_down_aggr
SET citus.log_remote_commands TO true;
SELECT ALL MIN((lower(CAST(test_collate_pushed_down_aggregate.a AS VARCHAR)) COLLATE "C"))
FROM ONLY test_collate_pushed_down_aggregate;
NOTICE: issuing SELECT min((lower(((a)::character varying COLLATE "default")) COLLATE "C")) AS min FROM ONLY collation_tests.test_collate_pushed_down_aggregate_20060004 test_collate_pushed_down_aggregate WHERE true
NOTICE: issuing SELECT min((lower(((a)::character varying)::text) COLLATE "C")) AS min FROM ONLY collation_tests.test_collate_pushed_down_aggregate_20060004 test_collate_pushed_down_aggregate WHERE true
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing SELECT min((lower(((a)::character varying COLLATE "default")) COLLATE "C")) AS min FROM ONLY collation_tests.test_collate_pushed_down_aggregate_20060005 test_collate_pushed_down_aggregate WHERE true
NOTICE: issuing SELECT min((lower(((a)::character varying)::text) COLLATE "C")) AS min FROM ONLY collation_tests.test_collate_pushed_down_aggregate_20060005 test_collate_pushed_down_aggregate WHERE true
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
min
---------------------------------------------------------------------
Expand Down