Skip to content

Comments

spatial key fix for null geometries#391

Merged
grandinj merged 5 commits intoh2database:masterfrom
svenschrader:master
Nov 8, 2016
Merged

spatial key fix for null geometries#391
grandinj merged 5 commits intoh2database:masterfrom
svenschrader:master

Conversation

@svenschrader
Copy link

this is the fix I suggested in the thread
https://groups.google.com/forum/#!topic/h2-database/S2H67G6BHPg

@grandinj
Copy link
Contributor

grandinj commented Nov 3, 2016

@nicolas-f care to look at this?

@svenschrader
Copy link
Author

Please observe how the edited TestSpatial fails without the changes in SpatialDataType. A larger table is required to reveal this error. See above mentioned thread

@nicolas-f
Copy link
Contributor

Hi,
Thanks for the PR
The spatial index should not be used by H2 when the query does not contain a spatial predicate. This way you do not have to insert null envelope in the RTree.

May be we have to update again the evaluation of spatial index cost here:
https://github.com/svenschrader/h2database/blob/master/h2/src/main/org/h2/index/SpatialTreeIndex.java#L187

@svenschrader
Copy link
Author

svenschrader commented Nov 7, 2016

Hi @nicolas-f
merci for your remark. You are right, setting some bogus envelope for the RTree is only a workaround.

However, the cost calculation you've mentioned does not seem to be an issue here(see below), BUT both the addRow and removeRow functions in MVSpatialIndex.java always iterate through all available indices (no matter what costs), and one of them is the Spatial one.

Please have a look at the updated MVTable, where null spatial keys for add and remove are ignored. It works on your TestSpatial unittest as well as on out local stresstests.

As for the query costs, they seem to be calculated when a SELECT by Geometry is performed. And null geometries won't be queried by this anyway.

What do you think?

@nicolas-f
Copy link
Contributor

Nice ! well done. Thank you 👍

@grandinj grandinj merged commit ba74733 into h2database:master Nov 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants