Thread
-
Fix misuse use of window_gettupleslot function (src/backend/executor/nodeWindowAgg.c)
Ranier Vilela <ranier.vf@gmail.com> — 2025-10-05T13:23:58Z
Hi. Per Coverity. CID 1635309: (#1 of 1): Unchecked return value (CHECKED_RETURN) 7. check_return: Calling window_gettupleslot without checking return value (as is done elsewhere 8 out of 9 times). The function "window_gettupleslot" can fail. All other calls check the return, In this case it could not be different. Fix by checking the return and reporting a message to the user, in case of failure. The error message, however, I'm not sure if it's ideal. best regards, Ranier Vilela
-
Re: Fix misuse use of window_gettupleslot function (src/backend/executor/nodeWindowAgg.c)
Tom Lane <tgl@sss.pgh.pa.us> — 2025-10-05T16:05:01Z
Ranier Vilela <ranier.vf@gmail.com> writes: > Per Coverity. > CID 1635309: (#1 of 1): Unchecked return value (CHECKED_RETURN) > 7. check_return: Calling window_gettupleslot without checking return value > (as is done elsewhere 8 out of 9 times). Yeah, the security team's Coverity instance just whined about that too. But isn't the correct behavior simply "return -1"? It looks to me like a failure should be interpreted as "row doesn't exist, therefore it's not in frame". What would be really useful is a test case that reaches this condition. That would make it plain what to do. regards, tom lane
-
Re: Fix misuse use of window_gettupleslot function (src/backend/executor/nodeWindowAgg.c)
Ranier Vilela <ranier.vf@gmail.com> — 2025-10-06T11:33:10Z
Em dom., 5 de out. de 2025 às 13:05, Tom Lane <tgl@sss.pgh.pa.us> escreveu: > Ranier Vilela <ranier.vf@gmail.com> writes: > > Per Coverity. > > CID 1635309: (#1 of 1): Unchecked return value (CHECKED_RETURN) > > 7. check_return: Calling window_gettupleslot without checking return > value > > (as is done elsewhere 8 out of 9 times). > > Yeah, the security team's Coverity instance just whined about that > too. But isn't the correct behavior simply "return -1"? It seems to me a better option. > It looks > to me like a failure should be interpreted as "row doesn't exist, > therefore it's not in frame". > I also believe that the original author did not expect a failure here. > What would be really useful is a test case that reaches this > condition. That would make it plain what to do. > There is a comment above that indicates that possibly a failure could also be the end of the partition. v1 patch attached. best regards, Ranier Vilela
-
Re: Fix misuse use of window_gettupleslot function (src/backend/executor/nodeWindowAgg.c)
Ranier Vilela <ranier.vf@gmail.com> — 2025-10-06T12:06:08Z
Em seg., 6 de out. de 2025 às 08:33, Ranier Vilela <ranier.vf@gmail.com> escreveu: > > Em dom., 5 de out. de 2025 às 13:05, Tom Lane <tgl@sss.pgh.pa.us> > escreveu: > >> Ranier Vilela <ranier.vf@gmail.com> writes: >> > Per Coverity. >> > CID 1635309: (#1 of 1): Unchecked return value (CHECKED_RETURN) >> > 7. check_return: Calling window_gettupleslot without checking return >> value >> > (as is done elsewhere 8 out of 9 times). >> >> Yeah, the security team's Coverity instance just whined about that >> too. But isn't the correct behavior simply "return -1"? > > It seems to me a better option. > > >> It looks >> to me like a failure should be interpreted as "row doesn't exist, >> therefore it's not in frame". >> > I also believe that the original author did not expect a failure here. > > >> What would be really useful is a test case that reaches this >> condition. That would make it plain what to do. >> > There is a comment above that indicates that possibly a failure could also > be the end of the partition. > > v1 patch attached. > It seems to me that this issue is being addressed in another thread. [1] I'll withdraw these patch. best regards, Ranier Vilela [1] https://www.postgresql.org/message-id/20251002.211550.1475922457918078317.ishii%40postgresql.org