Re: Removing unneeded self joins - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Removing unneeded self joins
Date
Msg-id 914330.1740330169@sss.pgh.pa.us
Whole thread Raw
In response to Removing unneeded self joins  (Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru>)
Responses Re: Removing unneeded self joins
List pgsql-hackers
Alexander Korotkov <aekorotkov@gmail.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
knownto 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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
Next
From: Daniel Gustafsson
Date:
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER