Opened 7 years ago

Closed 7 years ago

#3899 closed patch (fixed)

BTree opclass does not take object length into account

Reported by: komzpa Owned by: komzpa
Priority: medium Milestone: PostGIS 2.4.3
Component: postgis Version: 2.4.x
Keywords: Cc:

Description

BTree opclass has some issues:

  • in code hsz is used by mistake instead of bsz. That means it compares header size (thus bbox presence) instead of geometry length.
  • collections of EMPTY in different order and size get sorted in non-stable way;

Patch in https://github.com/postgis/postgis/pull/161

Change History (10)

comment:1 by strk, 7 years ago

Resolution: fixed
Status: newclosed

In 15975:

BTree sorting logic fix

Sort geometries that have different length but same prefix

  • allow fast path for geography points
  • fix gcc warnings
  • compare EMPTY geometries as normal memcmp path

Patch by Darafei Praliaskouski <me@…>

Fixes #3899

comment:2 by robe, 7 years ago

Resolution: fixed
Status: closedreopened

I think this is part of fix we were doing for 2.4.1 no? If so please backport and reclose or close and switch milestone to 2.5.0.

comment:3 by komzpa, 7 years ago

With this change, some people may need to rebuild their btrees (say on UNIQUE columns) once again. Without it, they will have corrupted uniqueness for some edge cases. Cases are quite obscure (a polygon that is ST_Reverse of itself can be sorted in wrong order if one copy of it has a box and another does not).

It just occured to me that we're still not looking enough into header - for two geometries with the geometry and different SRID we don't define a stable order. Looks like this thing is a candidate for oss-fuzz?

comment:4 by pramsey, 7 years ago

Milestone: PostGIS 2.4.1PostGIS 2.4.2

comment:5 by pramsey, 7 years ago

Close this?

comment:6 by pramsey, 7 years ago

Owner: changed from pramsey to komzpa
Status: reopenednew

comment:7 by pramsey, 7 years ago

This is an oldy but a goody,

select geometry_cmp('POINT(-0 0)'::geometry, 'POINT(0 0)'::GEometry);

 geometry_cmp 
--------------
            1

I noticed this when checking on whether we could have a hash index (#1014) to support certain CTE use cases. And lo, this old issue, which was never really resolved, came up. Do we consider POINT(-0 0) equal to POINT(0 0)? I guess not, actually, since we now consider LINESTRING(0 0, 1 1) to be different from LINESTRING(1 1, 0 0).

Last edited 7 years ago by pramsey (previous) (diff)

comment:8 by strk, 7 years ago

I don't think POINT(-0 0) is to POINT(0 0) what LINESTRING(0 0,1 1) is to LINESTRING(1 1,0 0). Anyway if the equality operator is "bitwise equality" then -0 != 0. What is equality (dejavu?)

comment:9 by pramsey, 7 years ago

Milestone: PostGIS 2.4.2PostGIS 2.4.3

comment:10 by pramsey, 7 years ago

Resolution: fixed
Status: newclosed

Ticket seems to have lost its mojo.

Note: See TracTickets for help on using tickets.