PG16 compatibility - outer join checks, subscription password, crash fixes (#7097)

PG16 compatibility - Part 4

Check out part 1 42d956888d
part 2 0d503dd5ac
part 3 907d72e60d

This commit is in the series of PG16 compatibility commits.
It adds some outer join checks to the planner,
the new password_required option to the subscription,
and a crash fix related to PGIOAlignedBlock, see below for more details:

- Fix PGIOAlignedBlock Assert crash in PG16 
Relevant PG commit:
faeedbcefd
faeedbcefd40bfdf314e048c425b6d9208896d90

- Pass planner info as argument to make_simple_restrictinfo 
Pre PG16 passing plannerInfo to make_simple_restrictinfo
was only needed for placeholder Vars, which is not the case
in this part of the codebase because we are building the
expression from shard intervals which don't have placeholder
vars.
However, PG16 is counting baserels appearing in clause_relids
and is deleting the rels mentioned in plannerinfo->outer_join_rels
Hence directly accessing plannerinfo.
We will crash if we leave it as NULL.
For reference
2489d76c49 (diff-e045c41eda9686451a7993e91518e40056b3739365e39eb1b70ae438dc1f7c76R207)
Relevant PG commit:
2489d76c49
2489d76c4906f4461a364ca8ad7e0751ead8aa0d

- Add outer join checks, root->simple_rel_array

- fix rebalancer to include passwork_required option 
Relevant PG commit:
c3afe8cf5a
c3afe8cf5a1e465bd71e48e4bc717f5bfdc7a7d6

More PG16 compatibility commits are coming soon ...
pull/7099/head
Naisila Puka 2023-08-04 14:51:28 +03:00 committed by GitHub
parent 907d72e60d
commit 7c6b4ce103
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 62 additions and 9 deletions

View File

@ -169,7 +169,11 @@ ColumnarStorageInit(SMgrRelation srel, uint64 storageId)
}
/* create two pages */
#if PG_VERSION_NUM >= PG_VERSION_16
PGIOAlignedBlock block;
#else
PGAlignedBlock block;
#endif
Page page = block.data;
/* write metapage */

View File

@ -901,15 +901,7 @@ RouterModifyTaskForShardInterval(Query *originalQuery,
continue;
}
/*
* passing NULL for plannerInfo will be problematic if we have placeholder
* vars. However, it won't be the case here because we are building
* the expression from shard intervals which don't have placeholder vars.
* Note that this is only the case with PG14 as the parameter doesn't exist
* prior to that.
*/
shardRestrictionList = make_simple_restrictinfo(NULL,
shardRestrictionList = make_simple_restrictinfo(restriction->plannerInfo,
(Expr *) shardOpExpressions);
extendedBaseRestrictInfo = lappend(extendedBaseRestrictInfo,
shardRestrictionList);

View File

@ -171,6 +171,8 @@ static bool FindQueryContainingRTEIdentityInternal(Node *node,
static int ParentCountPriorToAppendRel(List *appendRelList, AppendRelInfo *appendRelInfo);
static bool IsVarRelOptOuterJoin(PlannerInfo *root, Var *varToBeAdded);
/*
* AllDistributionKeysInQueryAreEqual returns true if either
@ -1238,6 +1240,12 @@ AddToAttributeEquivalenceClass(AttributeEquivalenceClass *attributeEquivalenceCl
return;
}
/* outer join checks in PG16 */
if (IsVarRelOptOuterJoin(root, varToBeAdded))
{
return;
}
RangeTblEntry *rangeTableEntry = root->simple_rte_array[varToBeAdded->varno];
if (rangeTableEntry->rtekind == RTE_RELATION)
{
@ -1379,6 +1387,30 @@ GetTargetSubquery(PlannerInfo *root, RangeTblEntry *rangeTableEntry, Var *varToB
}
/*
* IsVarRelOptOuterJoin returns true if the Var to be added
* is an outer join, false otherwise.
*/
static bool
IsVarRelOptOuterJoin(PlannerInfo *root, Var *varToBeAdded)
{
#if PG_VERSION_NUM >= PG_VERSION_16
if (root->simple_rel_array_size <= varToBeAdded->varno)
{
return true;
}
RelOptInfo *rel = root->simple_rel_array[varToBeAdded->varno];
if (rel == NULL)
{
/* must be an outer join */
return true;
}
#endif
return false;
}
/*
* AddUnionAllSetOperationsToAttributeEquivalenceClass recursively iterates on all the
* append rels, sets the varno's accordingly and adds the

View File

@ -1530,7 +1530,23 @@ CreateSubscriptions(MultiConnection *sourceConnection,
appendStringInfo(createSubscriptionCommand,
"CREATE SUBSCRIPTION %s CONNECTION %s PUBLICATION %s "
"WITH (citus_use_authinfo=true, create_slot=false, "
#if PG_VERSION_NUM >= PG_VERSION_16
/*
* password_required specifies whether connections to the publisher
* made as a result of this subscription must use password authentication.
* However, this setting is ignored when the subscription is owned
* by a superuser.
* Given that this command is executed below with superuser
* ExecuteCriticalRemoteCommand(target->superuserConnection,
* createSubscriptionCommand->data);
* We are safe to pass password_required as false because
* it will be ignored anyway
*/
"copy_data=false, enabled=false, slot_name=%s, password_required=false",
#else
"copy_data=false, enabled=false, slot_name=%s",
#endif
quote_identifier(target->subscriptionName),
quote_literal_cstr(conninfo->data),
quote_identifier(target->publication->name),

View File

@ -294,3 +294,12 @@ s/\/\*\{"cId":.*\*\///g
# Notice message that contains current columnar version that makes it harder to bump versions
s/(NOTICE: issuing CREATE EXTENSION IF NOT EXISTS citus_columnar WITH SCHEMA pg_catalog VERSION )"[0-9]+\.[0-9]+-[0-9]+"/\1 "x.y-z"/
# pg16 changes
# can be removed when dropping PG14&15 support
#if PG_VERSION_NUM < PG_VERSION_16
# (This is not preprocessor directive, but a reminder for the developer that will drop PG14&15 support )
s/, password_required=false//g
#endif /* PG_VERSION_NUM < PG_VERSION_16 */