Date: Mon, 06 Oct 2025 08:51:33 +0900 (JST)
Message-Id: <20251006.085133.920304905412132984.ishii@postgresql.org>
To: tgl@sss.pgh.pa.us
Cc: ojford@gmail.com, peter@eisentraut.org, li.evan.chao@gmail.com,
 krasiyan@gmail.com, vik@postgresfriends.org, andrew@tao11.riddles.org.uk,
 david@fetter.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: <1686755.1759679957@sss.pgh.pa.us>
References: 
 <CAGMVOdvUp_SPVbta9HOGx3YbGBs_+3VSZh0Fu-C=9WdR_yqzRQ@mail.gmail.com>
	<20251003.100606.2086363717963832725.ishii@postgresql.org>
	<1686755.1759679957@sss.pgh.pa.us>
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Mon_Oct__6_08_51_33_2025_280)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20251006.085133.920304905412132984.ishii%40postgresql.org>
Precedence: bulk

----Next_Part(Mon_Oct__6_08_51_33_2025_280)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

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

----Next_Part(Mon_Oct__6_08_51_33_2025_280)--
Content-Type: Application/Octet-Stream
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="v1-0001-Fix-Coverity-issues-reported-in-commit-25a30bbd42.patch"

RnJvbSAyOGY1MzMyYjUwYTdjZjExZjE2ZDM1YjI4NjA2MjhlNTgwNzdjMjM5IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBUYXRzdW8gSXNoaWkgPGlzaGlpQHBvc3RncmVzcWwub3JnPgpE
YXRlOiBNb24sIDYgT2N0IDIwMjUgMDg6NDU6MDMgKzA5MDAKU3ViamVjdDogW1BBVENIIHYxXSBG
aXggQ292ZXJpdHkgaXNzdWVzIHJlcG9ydGVkIGluIGNvbW1pdCAyNWEzMGJiZDQyMy4KClRoaXMg
Y29tbWl0IGZpeGVzIHNldmVyYWwgaXNzdWVzIHBvaW50ZWQgb3V0IGJ5IENvdmVyaXR5LgoKLSBJ
biByb3dfaXNfaW5fZnJhbWUoKSwgcmV0dXJuIHZhbHVlIG9mIHdpbmRvd19nZXR0dXBsZXNsb3Qo
KSB3YXMgbm90IGNoZWNrZWQuCgotIFdpbkdldEZ1bmNBcmdJblBhcnRpdGlvbigpIHRyaWVkIHRv
IGRlcmVmZmVyZW5jZSAiaXNvdXQiIHBvaW50ZXIKIGV2ZW4gaWYgaXQncyBOVUxMIGluIHNvbWUg
cGxhY2VzLgoKRGlzY3Vzc2lvbjogaHR0cHM6Ly9wb3N0Z3IuZXMvbS8yMDI1MTAwNTE2MTIuZ3c2
N2psYzJpcXB3JTQwYWx2aGVycmUucGdzcWwKLS0tCiBzcmMvYmFja2VuZC9leGVjdXRvci9ub2Rl
V2luZG93QWdnLmMgfCAxMSArKysrKystLS0tLQogMSBmaWxlIGNoYW5nZWQsIDYgaW5zZXJ0aW9u
cygrKSwgNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9leGVjdXRvci9u
b2RlV2luZG93QWdnLmMgYi9zcmMvYmFja2VuZC9leGVjdXRvci9ub2RlV2luZG93QWdnLmMKaW5k
ZXggY2Y2NjdjODEyMTEuLjdjY2YxMTYwZGNhIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9leGVj
dXRvci9ub2RlV2luZG93QWdnLmMKKysrIGIvc3JjL2JhY2tlbmQvZXhlY3V0b3Ivbm9kZVdpbmRv
d0FnZy5jCkBAIC0xNTAxLDggKzE1MDEsOSBAQCByb3dfaXNfaW5fZnJhbWUoV2luZG93T2JqZWN0
IHdpbm9iaiwgaW50NjQgcG9zLCBUdXBsZVRhYmxlU2xvdCAqc2xvdCwKIAkJCS8qIGZvbGxvd2lu
ZyByb3cgdGhhdCBpcyBub3QgcGVlciBpcyBvdXQgb2YgZnJhbWUgKi8KIAkJCWlmIChwb3MgPiB3
aW5zdGF0ZS0+Y3VycmVudHBvcykKIAkJCXsKLQkJCQlpZiAoZmV0Y2hfdHVwbGUpCi0JCQkJCXdp
bmRvd19nZXR0dXBsZXNsb3Qod2lub2JqLCBwb3MsIHNsb3QpOworCQkJCWlmIChmZXRjaF90dXBs
ZSkJLyogbmVlZCB0byBmZXRjaCB0dXBsZT8gKi8KKwkJCQkJaWYgKCF3aW5kb3dfZ2V0dHVwbGVz
bG90KHdpbm9iaiwgcG9zLCBzbG90KSkKKwkJCQkJCXJldHVybiAtMTsKIAkJCQlpZiAoIWFyZV9w
ZWVycyh3aW5zdGF0ZSwgc2xvdCwgd2luc3RhdGUtPnNzLnNzX1NjYW5UdXBsZVNsb3QpKQogCQkJ
CQlyZXR1cm4gLTE7CiAJCQl9CkBAIC0zNzYxLDcgKzM3NjIsNyBAQCBXaW5HZXRGdW5jQXJnSW5Q
YXJ0aXRpb24oV2luZG93T2JqZWN0IHdpbm9iaiwgaW50IGFyZ25vLAogCXsKIAkJZGF0dW0gPSBn
ZXR0dXBsZV9ldmFsX3BhcnRpdGlvbih3aW5vYmosIGFyZ25vLAogCQkJCQkJCQkJCWFic19wb3Ms
IGlzbnVsbCwgaXNvdXQpOwotCQlpZiAoISppc291dCAmJiBzZXRfbWFyaykKKwkJaWYgKGlzb3V0
ICYmICEqaXNvdXQgJiYgc2V0X21hcmspCiAJCQlXaW5TZXRNYXJrUG9zaXRpb24od2lub2JqLCBh
YnNfcG9zKTsKIAkJcmV0dXJuIGRhdHVtOwogCX0KQEAgLTM4MDMsNyArMzgwNCw3IEBAIFdpbkdl
dEZ1bmNBcmdJblBhcnRpdGlvbihXaW5kb3dPYmplY3Qgd2lub2JqLCBpbnQgYXJnbm8sCiAJCQlk
ZWZhdWx0OgkJCS8qIG5lZWQgdG8gY2hlY2sgTlVMTCBvciBub3QgKi8KIAkJCQlkYXR1bSA9IGdl
dHR1cGxlX2V2YWxfcGFydGl0aW9uKHdpbm9iaiwgYXJnbm8sCiAJCQkJCQkJCQkJCQlhYnNfcG9z
LCBpc251bGwsIGlzb3V0KTsKLQkJCQlpZiAoKmlzb3V0KQkJLyogb3V0IG9mIHBhcnRpdGlvbj8g
Ki8KKwkJCQlpZiAoaXNvdXQgJiYgKmlzb3V0KQkvKiBvdXQgb2YgcGFydGl0aW9uPyAqLwogCQkJ
CQlyZXR1cm4gZGF0dW07CiAKIAkJCQlpZiAoISppc251bGwpCkBAIC0zODE0LDcgKzM4MTUsNyBA
QCBXaW5HZXRGdW5jQXJnSW5QYXJ0aXRpb24oV2luZG93T2JqZWN0IHdpbm9iaiwgaW50IGFyZ25v
LAogCQl9CiAJfSB3aGlsZSAobm90bnVsbF9vZmZzZXQgPCBub3RudWxsX3JlbHBvcyk7CiAKLQlp
ZiAoISppc291dCAmJiBzZXRfbWFyaykKKwlpZiAoaXNvdXQgJiYgISppc291dCAmJiBzZXRfbWFy
aykKIAkJV2luU2V0TWFya1Bvc2l0aW9uKHdpbm9iaiwgYWJzX3Bvcyk7CiAKIAlyZXR1cm4gZGF0
dW07Ci0tIAoyLjQzLjAKCg==

----Next_Part(Mon_Oct__6_08_51_33_2025_280)----