public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tatsuo Ishii <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
Date: Mon, 06 Oct 2025 08:51:33 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAGMVOdvUp_SPVbta9HOGx3YbGBs_+3VSZh0Fu-C=9WdR_yqzRQ@mail.gmail.com>
<[email protected]>
<[email protected]>
Thank you for the report!
> Coverity is not very happy with this patch.
> It's complaining that the result of window_gettupleslot
> is not checked, which seems valid:
>
> 1503 {
> 1504 if (fetch_tuple)
>>>> CID 1666587: Error handling issues (CHECKED_RETURN)
>>>> Calling "window_gettupleslot" without checking return value (as is done elsewhere 8 out of 9 times).
> 1505 window_gettupleslot(winobj, pos, slot);
> 1506 if (!are_peers(winstate, slot, winstate->ss.ss_ScanTupleSlot))
> 1507 return -1;
Yes, I forgot to check the return value of window_gettupleslot.
> and also that WinGetFuncArgInPartition is dereferencing
> a possibly-null "isout" pointer at several places, including
>
>>>> Dereferencing null pointer "isout".
> 3806 if (*isout) /* out of partition? */
>
>>>> Dereferencing null pointer "isout".
> 3817 if (!*isout && set_mark)
> 3818 WinSetMarkPosition(winobj, abs_pos);
>
>>>> Dereferencing null pointer "isout".
> 3817 if (!*isout && set_mark)
> 3818 WinSetMarkPosition(winobj, abs_pos);
>
> The latter complaints seem to be because some places in
> WinGetFuncArgInPartition check for nullness of that pointer
> and some do not. That looks like at least a latent bug
> to me.
Agreed.
Attached is a patch to fix the issue.
Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
Attachments:
[application/octet-stream] v1-0001-Fix-Coverity-issues-reported-in-commit-25a30bbd42.patch (2.2K, ../[email protected]/2-v1-0001-Fix-Coverity-issues-reported-in-commit-25a30bbd42.patch)
download | inline diff:
From 28f5332b50a7cf11f16d35b2860628e58077c239 Mon Sep 17 00:00:00 2001
From: Tatsuo Ishii <[email protected]>
Date: Mon, 6 Oct 2025 08:45:03 +0900
Subject: [PATCH v1] Fix Coverity issues reported in commit 25a30bbd423.
This commit fixes several issues pointed out by Coverity.
- In row_is_in_frame(), return value of window_gettupleslot() was not checked.
- WinGetFuncArgInPartition() tried to derefference "isout" pointer
even if it's NULL in some places.
Discussion: https://postgr.es/m/202510051612.gw67jlc2iqpw%40alvherre.pgsql
---
src/backend/executor/nodeWindowAgg.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index cf667c81211..7ccf1160dca 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -1501,8 +1501,9 @@ row_is_in_frame(WindowObject winobj, int64 pos, TupleTableSlot *slot,
/* following row that is not peer is out of frame */
if (pos > winstate->currentpos)
{
- if (fetch_tuple)
- window_gettupleslot(winobj, pos, slot);
+ if (fetch_tuple) /* need to fetch tuple? */
+ if (!window_gettupleslot(winobj, pos, slot))
+ return -1;
if (!are_peers(winstate, slot, winstate->ss.ss_ScanTupleSlot))
return -1;
}
@@ -3761,7 +3762,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno,
{
datum = gettuple_eval_partition(winobj, argno,
abs_pos, isnull, isout);
- if (!*isout && set_mark)
+ if (isout && !*isout && set_mark)
WinSetMarkPosition(winobj, abs_pos);
return datum;
}
@@ -3803,7 +3804,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno,
default: /* need to check NULL or not */
datum = gettuple_eval_partition(winobj, argno,
abs_pos, isnull, isout);
- if (*isout) /* out of partition? */
+ if (isout && *isout) /* out of partition? */
return datum;
if (!*isnull)
@@ -3814,7 +3815,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno,
}
} while (notnull_offset < notnull_relpos);
- if (!*isout && set_mark)
+ if (isout && !*isout && set_mark)
WinSetMarkPosition(winobj, abs_pos);
return datum;
--
2.43.0
view thread (91+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox