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

Em qua., 27 de set. de 2023 às 22:28, David Rowley <dgrowleyml(at)gmail(dot)com>
escreveu:

> On Thu, 28 Sept 2023 at 02:37, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> >> 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."
> >
> > Forgive my impulsiveness, anyone who loves perfect, well written code,
> would understand.
>
> Perhaps, but the committers on this project seem to be against the
> blunderbuss approach to committing patches. You might meet less
> resistance around here if you assume all of their weapons have a scope
> and that they like to aim for something before pulling the trigger.
>
Perhaps, and using your own words, the leaders on this project seem
to be against reviewers armed with blunderbuss, too.

>
> Personally, this seems like a good idea to me and I'd like to follow
> it too. If you'd like to hold off you a committer whose weapon of
> choice is the blunderbuss then I can back off and let you wait for one
> to come along. Just let me know.
>
Please, no, I love good combat too.
You are welcome in my threads.
But I hope that you will be strong like me, and don't wait for weak
comebacks,
when you find another strong elephant.

> > Do you have an objection to fixing the function
> find_base_rel_ignore_join?
> > Or is it included in unrelated changes?
>
> Well, the topic seems to be adding additional safety to prevent
> accessing negative entries for simple_rel_array. I can't think why
> fixing the same theoretical hazard in find_base_rel_ignore_join()
> would be unrelated.

Good to know.

> I hope you can see the difference here. Randomly
> adjusting function signatures because you happen to be changing some
> code within that function does not, in my book, seem related.

I confess that some "in pass", "while there", and some desire to enrich the
patch, clouded my judgment.

So it seems that we have some consensus, I propose version 2 of the patch,
which I hope we will have to correct, perhaps, the words of the comments.

best regards,
Ranier Vilela