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

On Fri, 29 Sept 2023 at 01:00, Ranier Vilela wrote: > Perhaps, and using your own words, the leaders on this project seem > to be against reviewers armed with blunderbuss, too. I don't have any ideas on what you're talking about here, but if this is a valid concern that you think is unfair then m

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 escreveu: > On Thu, 28 Sept 2023 at 02:37, Ranier Vilela 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

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

On Thu, 28 Sept 2023 at 02:37, Ranier Vilela 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 nece

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

At 2023-09-27 10:37:45 -0300, ranier...@gmail.com wrote: > > Forgive my impulsiveness, anyone who loves perfect, well written code, > would understand. I actually find this characterisation offensive. Being scrupulous about not grouping random drive-by changes together with the primary change is

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

Em qua., 27 de set. de 2023 às 04:35, David Rowley escreveu: > On Wed, 27 Sept 2023 at 06:10, Ranier Vilela 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

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

On Wed, 27 Sept 2023 at 06:10, Ranier Vilela 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, > whic

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

Em ter., 26 de set. de 2023 às 09:30, Ranier Vilela escreveu: > Em ter., 26 de set. de 2023 às 07:34, Ashutosh Bapat < > ashutosh.bapat@gmail.com> escreveu: > >> On Tue, Sep 26, 2023 at 3:32 PM David Rowley >> wrote: >> > >> > find_base_rel() could be made more robust for free by just castin

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

On Wed, 27 Sept 2023 at 01:31, Ranier Vilela wrote: > It seems to me that it adds a LEA instruction. > https://godbolt.org/z/b4jK3PErE There's a fairly significant difference in the optimisability of a comparison with a compile-time constant vs a variable. For example, would you expect the compil

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

Em ter., 26 de set. de 2023 às 07:34, Ashutosh Bapat < ashutosh.bapat@gmail.com> escreveu: > On Tue, Sep 26, 2023 at 3:32 PM David Rowley wrote: > > > > find_base_rel() could be made more robust for free by just casting the > > relid and simple_rel_array_size to uint32 while checking that rel

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

On Tue, Sep 26, 2023 at 3:32 PM David Rowley wrote: > > find_base_rel() could be made more robust for free by just casting the > relid and simple_rel_array_size to uint32 while checking that relid < > root->simple_rel_array_size. The 0th element should be NULL anyway, > so "if (rel)" should let r

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

On Tue, 26 Sept 2023 at 21:45, Ashutosh Bapat wrote: > However, I agree that changing find_base_rel() the way you have done > in your patch is fine and mildly future-proof. +1 to that idea > irrespective of what bitmapset functions do. I'm not a fan of adding additional run-time overhead for this

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

On Mon, Sep 25, 2023 at 7:14 PM Ranier Vilela wrote: > > Em seg., 25 de set. de 2023 às 08:23, Ashutosh Bapat > escreveu: >> >> On Sat, Sep 23, 2023 at 7:29 PM Ranier Vilela wrote: >> > >> > Hi, >> > >> > Per Coverity. >> > CID 1518088 (#2 of 2): Improper use of negative value (NEGATIVE_RETURNS

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

Em seg., 25 de set. de 2023 às 08:23, Ashutosh Bapat < ashutosh.bapat@gmail.com> escreveu: > On Sat, Sep 23, 2023 at 7:29 PM Ranier Vilela wrote: > > > > Hi, > > > > Per Coverity. > > CID 1518088 (#2 of 2): Improper use of negative value (NEGATIVE_RETURNS) > > > > The function bms_singleton_m

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

On Sat, Sep 23, 2023 at 7:29 PM Ranier Vilela wrote: > > Hi, > > Per Coverity. > CID 1518088 (#2 of 2): Improper use of negative value (NEGATIVE_RETURNS) > > The function bms_singleton_member can returns a negative number. > > /* > * Get a child rel for rel2 with the relids. See above comments. >

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

Hi, On Sat, Sep 23, 2023 at 9:59 PM Ranier Vilela wrote: > Per Coverity. > CID 1518088 (#2 of 2): Improper use of negative value (NEGATIVE_RETURNS) > > The function bms_singleton_member can returns a negative number. > > /* > * Get a child rel for rel2 with the relids. See above comments. > */ >

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

Hi, Per Coverity. CID 1518088 (#2 of 2): Improper use of negative value (NEGATIVE_RETURNS) The function bms_singleton_member can returns a negative number. /* * Get a child rel for rel2 with the relids. See above comments. */ if (rel2_is_simple) { int varno = bms_singleton_member(child_relids2)