pgsql: Fix netmask handling in inet_minmax_multi_ops

Started by Tomas Vondraover 3 years ago3 messagescomitters
Jump to latest
#1Tomas Vondra
tomas.vondra@2ndquadrant.com

Fix netmask handling in inet_minmax_multi_ops

When calculating distance in brin_minmax_multi_distance_inet(), the
netmask was applied incorrectly. This results in (seemingly) incorrect
ordering of values, triggering an assert.

For builds without asserts this is mostly harmless - we may merge other
ranges, possibly resulting in slightly less efficient index. But it's
still correct and the greedy algorithm doesn't guarantee optimality
anyway.

Backpatch to 14, where minmax-multi indexes were introduced.

Reported by Dmitry Dolgov, investigation and fix by me.

Reported-by: Dmitry Dolgov
Backpatch-through: 14
Discussion: /messages/by-id/17774-c6f3e36dd4471e67@postgresql.org

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/e8583126833a53f4eebe28a8de45d128f01ff664

Modified Files
--------------
src/backend/access/brin/brin_minmax_multi.c | 4 ++--
src/test/regress/expected/brin_multi.out | 6 ++++++
src/test/regress/sql/brin_multi.sql | 7 +++++++
3 files changed, 15 insertions(+), 2 deletions(-)

#2Tomas Vondra
tomas.vondra@2ndquadrant.com
In reply to: Tomas Vondra (#1)
Re: pgsql: Fix netmask handling in inet_minmax_multi_ops

On 3/20/23 10:28, Tomas Vondra wrote:

Fix netmask handling in inet_minmax_multi_ops

When calculating distance in brin_minmax_multi_distance_inet(), the
netmask was applied incorrectly. This results in (seemingly) incorrect
ordering of values, triggering an assert.

For builds without asserts this is mostly harmless - we may merge other
ranges, possibly resulting in slightly less efficient index. But it's
still correct and the greedy algorithm doesn't guarantee optimality
anyway.

Backpatch to 14, where minmax-multi indexes were introduced.

Reported by Dmitry Dolgov, investigation and fix by me.

Reported-by: Dmitry Dolgov

Correction - the issue was reported by Robins Tharakan, I got confused
while writing the commit message. I don't know if this issue is to be
mentioned in release notes (considering it mostly affects just assert
builds), but if we do we should the correct name.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tomas Vondra (#2)
Re: pgsql: Fix netmask handling in inet_minmax_multi_ops

Tomas Vondra <tomas.vondra@enterprisedb.com> writes:

On 3/20/23 10:28, Tomas Vondra wrote:

Reported by Dmitry Dolgov, investigation and fix by me.
Reported-by: Dmitry Dolgov

Correction - the issue was reported by Robins Tharakan, I got confused
while writing the commit message. I don't know if this issue is to be
mentioned in release notes (considering it mostly affects just assert
builds), but if we do we should the correct name.

We don't normally cite the bug reporter's name in release notes,
just the person(s) who wrote the patch. So this won't be an issue
for the notes.

regards, tom lane