pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. 6+ messages / 1 participants [nested] [flat]
* pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. @ 2025-04-12 16:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-04-12 16:28 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix GIN's shimTriConsistentFn to not corrupt its input. Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/e708ffe79df07fe26801869f391f4c227615dd42 Modified Files -------------- contrib/intarray/expected/_int.out | 42 ++++++++++++++++++++++++++++++++++++++ contrib/intarray/sql/_int.sql | 7 +++++++ src/backend/access/gin/ginlogic.c | 20 +++++++++++++----- 3 files changed, 64 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. @ 2025-04-12 16:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-04-12 16:28 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix GIN's shimTriConsistentFn to not corrupt its input. Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/8c153fcfa0d6281f4d2754dfdc74055aee86814e Modified Files -------------- contrib/intarray/expected/_int.out | 42 ++++++++++++++++++++++++++++++++++++++ contrib/intarray/sql/_int.sql | 7 +++++++ src/backend/access/gin/ginlogic.c | 20 +++++++++++++----- 3 files changed, 64 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. @ 2025-04-12 16:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-04-12 16:28 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix GIN's shimTriConsistentFn to not corrupt its input. Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/4b65b085aff2796f01aef7df053781ef6ab74752 Modified Files -------------- contrib/intarray/expected/_int.out | 42 ++++++++++++++++++++++++++++++++++++++ contrib/intarray/sql/_int.sql | 7 +++++++ src/backend/access/gin/ginlogic.c | 20 +++++++++++++----- 3 files changed, 64 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. @ 2025-04-12 16:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-04-12 16:28 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix GIN's shimTriConsistentFn to not corrupt its input. Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/9a8c16aeccaddd7db2b7faada1e507b0a9b15c45 Modified Files -------------- contrib/intarray/expected/_int.out | 42 ++++++++++++++++++++++++++++++++++++++ contrib/intarray/sql/_int.sql | 7 +++++++ src/backend/access/gin/ginlogic.c | 20 +++++++++++++----- 3 files changed, 64 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. @ 2025-04-12 16:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-04-12 16:28 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix GIN's shimTriConsistentFn to not corrupt its input. Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13 Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/1b47a112ac41b6414eadebcb3d5f3908bf440565 Modified Files -------------- contrib/intarray/expected/_int.out | 42 ++++++++++++++++++++++++++++++++++++++ contrib/intarray/sql/_int.sql | 7 +++++++ src/backend/access/gin/ginlogic.c | 20 +++++++++++++----- 3 files changed, 64 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. @ 2025-04-12 16:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2025-04-12 16:28 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix GIN's shimTriConsistentFn to not corrupt its input. Commit 0f21db36d made an assumption that GIN triConsistentFns would not modify their input entryRes[] arrays. But in fact, the "shim" triConsistentFn that we use for opclasses that don't supply their own did exactly that, potentially leading to wrong answers from a GIN index search. Through bad luck, none of the test cases that we have for such opclasses exposed the bug. One response to this could be that the assumption of consistency check functions not modifying entryRes[] arrays is a bad one, but it still seems reasonable to me. Notably, shimTriConsistentFn is itself assuming that with respect to the underlying boolean consistentFn, so it's sure being self-centered in supposing that it gets to do so. Fortunately, it's quite simple to fix shimTriConsistentFn to restore the entry-time state of entryRes[], so let's do that instead. This issue doesn't affect any core GIN opclasses, since they all supply their own triConsistentFns. It does affect contrib modules btree_gin, hstore, and intarray. Along the way, I (tgl) noticed that shimTriConsistentFn failed to pick up on a "recheck" flag returned by its first call to the boolean consistentFn. This may be only a latent problem, since it would be unlikely for a consistentFn to set recheck for the all-false case and not any other cases. (Indeed, none of our contrib modules do that.) Nonetheless, it's formally wrong. Reported-by: Vinod Sridharan <vsridh90@gmail.com> Author: Vinod Sridharan <vsridh90@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com Backpatch-through: 13 Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/c7597a1d369c05842db1b9a73b4ac76fe5a92a75 Modified Files -------------- contrib/intarray/expected/_int.out | 42 ++++++++++++++++++++++++++++++++++++++ contrib/intarray/sql/_int.sql | 7 +++++++ src/backend/access/gin/ginlogic.c | 20 +++++++++++++----- 3 files changed, 64 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-04-12 16:28 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-04-12 16:28 pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. Tom Lane <tgl@sss.pgh.pa.us> 2025-04-12 16:28 pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. Tom Lane <tgl@sss.pgh.pa.us> 2025-04-12 16:28 pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. Tom Lane <tgl@sss.pgh.pa.us> 2025-04-12 16:28 pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. Tom Lane <tgl@sss.pgh.pa.us> 2025-04-12 16:28 pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. Tom Lane <tgl@sss.pgh.pa.us> 2025-04-12 16:28 pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input. Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox