Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #29630 spatialindex for NaN points #29997

Merged
merged 1 commit into from
May 30, 2019

Conversation

PeterPetrik
Copy link
Contributor

fix #29630

@PeterPetrik PeterPetrik added backport release-3_4 Bug Either a bug report, or a bug fix. Let's hope for the latter! labels May 29, 2019
@nyalldawson
Copy link
Collaborator

Won't we need a similar fix in QgsSpatialIndex also?

@PeterPetrik
Copy link
Contributor Author

@nyalldawson fixed. crash with spatialindex can be reproduced by

  1. load the test file from the issue
  2. run in python console
l = iface.activeLayer()
i = QgsSpatialIndex(l)
f = l.getFeature(97)
i.deleteFeature(f)

@nyalldawson
Copy link
Collaborator

So now we just need some tests :)

@PeterPetrik
Copy link
Contributor Author

@nyalldawson I tried to replicated the crash in the test system with both manually-prepared features as well as the layer from the issue without success. I can add some test with some random NaN numbers to the suite, but I am not sure how it represents the bug. The bug occurs only if the RTree is specifically filled with a set of NaN points and valid points with certain balancing. And also you need to remove specific fid from to tree to crash. Also you cannot replicate it on Linux at all (regardless of the version) so travis will not catch it anyways. Are you OK with not adding tests or should I add some test that just tests some random NaN points, but not the bug as such?

@nyalldawson
Copy link
Collaborator

Ok, that's fair enough!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing a GeoPackage feature with spaces in the field names can crash QGIS
3 participants