Re: Removing unneeded self joins

Alexander Korotkov <aekorotkov(at)gmail(dot)com> writes:
> I've corrected some spelling error reported by Alexander Lakhin
> privately to me. Also, I've revised comments around ChangeVarNodes()
> and ChangeVarNodesExtended(). I'm going to continue nitpicking this
> patch during next couple days then push it if no objections.

Coverity has another nit-pick:

/srv/coverity/git/pgsql-git/postgresql/src/backend/optimizer/plan/analyzejoins.c: 327 in remove_rel_from_query()
321 static void
322 remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
323 int subst, SpecialJoinInfo *sjinfo,
324 Relids joinrelids)
325 {
326 int relid = rel->relid;
>>> CID 1643155: Integer handling issues (INTEGER_OVERFLOW)
>>> Expression "ojrelid", which is equal to 4294967295, where "(sjinfo != NULL) ? sjinfo->ojrelid : 4294967295U" is known to be equal to 4294967295, overflows the type that receives it, a signed integer 32 bits wide.
327 int ojrelid = (sjinfo != NULL) ? sjinfo->ojrelid : -1;
328 Index rti;
329 ListCell *l;
330
331 /*
332 * Update all_baserels and related relid sets.

This is unhappy because sjinfo->ojrelid, which is declared Index
(that is, unsigned int) is being converted to int. Admittedly
there are plenty of other places that do likewise, but the additional
load of assuming that -1 isn't a possible value of sjinfo->ojrelid
seems to be enough to draw its ire.

I suggest finding another way to code this function that doesn't
depend on that type pun. I think it's fairly accidental that
adjust_relid_set doesn't misbehave on -1 anyway, so personally I'd
get rid of that local variable entirely in favor of something like

if (sjinfo != NULL)
{
root->outer_join_rels = adjust_relid_set(root->outer_join_rels,
sjinfo->ojrelid, subst);
root->all_query_rels = adjust_relid_set(root->all_query_rels,
sjinfo->ojrelid, subst);
}

regards, tom lane