Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

On Wed, 27 Sept 2023 at 06:10, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> As suggested, casting is the best option that does not add any overhead and improves the robustness of the find_base_rel function.
> I propose patch v1, with the additional addition of fixing the find_base_rel_ignore_join function,
> which despite not appearing in Coverity reports, suffers from the same problem.

Can you confirm that this silences the Converity warning?

I think it probably warrants a comment to mention why we cast to uint32.

e.g. /* perform an unsigned comparison so that we also catch negative
relid values */

> Taking advantage, I also propose a scope reduction,
> as well as the const of the root parameter, which is very appropriate.

Can you explain why adding the const qualifier is "very appropriate"
to catching negative relids?

Please check [1] for the mention of:

"The fastest way to get your patch rejected is to make unrelated
changes. Reformatting lines that haven't changed, changing unrelated
comments you felt were poorly worded, touching code not necessary to
your change, etc. Each patch should have the minimum set of changes
required to work robustly. If you do not follow the code formatting
suggestions above, expect your patch to be returned to you with the
feedback of "follow the code conventions", quite likely without any
other review."

David

[1] https://wiki.postgresql.org/wiki/Submitting_a_Patch