Alexander Korotkov <aekorot...@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 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