Closed qgib closed 5 years ago
qgib
commented
10 years ago Author Name: Giovanni Manghi (@gioman)
Hi Saber, is this a SAGA issue or a QGIS one? What happens if you do the same using native SAGA instead of Processing?
qgib
commented
10 years ago Author Name: Saber Razmjooei (@saberraz)
Gio, I suppose it is a problem with SAGA. But will be to have it fixed upstream or work-around in QGIS. Otherwise, it might confuse users.
qgib
commented
10 years ago Author Name: Victor Olaya (@volaya)
This is really weird. If you have modified the shapefile and deleted data...how can SAGA know that there was a feature in there that now is missing?? If you have changed the shapefile, there is no way the deleted feature can be recovered.
Maybe you are saving to a different file and then using the original to call SAGA?
qgib
commented
10 years ago Author Name: Saber Razmjooei (@saberraz)
Victor,
I guess in QGIS, when you delete a feature in a shapefile, it does not physically delete that feature. It flags it as deleted but it is still there.
The shapefile gets "cleaned" when you save as...
qgib
commented
10 years ago Author Name: Giovanni Manghi (@gioman)
Saber Razmjooei wrote:
Victor,
I guess in QGIS, when you delete a feature in a shapefile, it does not physically delete that feature. It flags it as deleted but it is still there.
The shapefile gets "cleaned" when you save as...
Hi Saber, this does not seems to be the case. Delete a feature, save edits and load the shape in another QGIS window (or any other GIS package), the feature is not there.
qgib
commented
10 years ago Author Name: Saber Razmjooei (@saberraz)
Sorry Gio, that's correct. The bug only happens in active QGIS window, where editing took place.
qgib
commented
9 years ago Author Name: Victor Olaya (@volaya)
Saber
Can you confirm the QGIS version you are using? I am finding that behaviour (deleted features that are actually not deleted in the shapefile, even after closing the edition), not just in Processing, but in QGIS in general. Taking the modified layer to a different software (as Processing does when passing the layer to SAGA) shows the deleted features. But I see this issue only in 2.4, not in 2.2. IF this is the case, then it's a QGIS issue and we should open another ticket.
qgib
commented
9 years ago Author Name: Victor Olaya (@volaya)
IT seems DBF files have a deleted flag (http://www.clicketyclick.dk/databases/xbase/format/dbf.html#DBF_NOTE_9_TARGET). SAGA might not recognise it, and probably the way QGIS was removing features before was just actually eliminating them, while now it sets that flag to deleted. That would explain the error.
qgib
commented
9 years ago Author Name: Saber Razmjooei (@saberraz)
Victor, it is in 2.4. As you said the problem is not limited only to the Processing. The deleted features appear in Vector > Geoprocessing results too.
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
see also #19592
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Victor Olaya wrote: But I see this issue only in 2.4, not in 2.2. IF this is the case, then it's a QGIS issue and we should open another ticket.
it affects also 2.2 and previous qgis releases (and master).
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Saber Razmjooei wrote:
Victor, it is in 2.4. As you said the problem is not limited only to the Processing. The deleted features appear in Vector > Geoprocessing results too.
Hi Saber, while I confirm it affects also other software packages I cannot confirm that affects the own QGIS geoprocessing tools.
See also #19680
qgib
commented
9 years ago Author Name: Saber Razmjooei (@saberraz)
Gio, My bad, you are right. It works even in edit mode, when your changes still have not been saved!
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
In #19680 there are example of software packages affected bu this issue, anyway SAGA it is also affected and it is easy to test as it comes with QGIS.
A few notes:
a) after deleting a feature using the shape as input for SAGA gives
error: DBase file could not be opened.
removing and re-adding the shape and running it against SAGA again works without errors, but the operation (ex: buffer) takes into account also the features that it should have been deleted
b) using the node tool to edit a feature and using the shape as input in SAGA gives
error: DBase file could not be opened.
removing and re-adding the shape and running it against SAGA again works without errors and as expected
c) using the reshape tool to edit a feature and using the shape as input in SAGA gives
error: DBase file could not be opened.
removing and re-adding the shape and running it against SAGA again works without errors, but the operation (ex: buffer) result is as it was run against the input before being edited in qgis.
d) using the split features tool to edit a feature and using the shape as input in SAGA gives
error: corrupted shapefile.
removing and re-adding the shape does not help.
It is not strictly a regression, but given the huge interoperability issues that this issue creates I would like to ask to raise this as blocker.
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
I made some tests with ArcGIS 10:
a) delete a feature, save. Add the shape into arcgis, the deleted feature is there (both geometry and attribute).
so this confirms (again) #19592
Using the shape as input in arcgis (ex: buffer) then the "phantom" feature is removed from input and output is as expected.
doing operations as b) c) d) does not seems to have any bad effect when adding the shape in arcgis
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
it is being very hard to find a clear pattern:
there are shapes that when edited always causes SAGA (again, we are using SAGA in first place because it is easy available and works together with QGIS) to use also the edited/deleted features or to throw an error (error: corrupted shapefile).
other sw packages seems affected also in the cases when SAGA it is not.
the only thing that seems to work almost all the times is to re-save the shapefile as a copy.
qgib
commented
9 years ago Author Name: Jukka Rahkonen (Jukka Rahkonen)
I was reading this stackexchange question http://gis.stackexchange.com/questions/118689/cant-transform-lines-to-polygons with a sample data available for a few days now at http://dropcanvas.com/7i4oq. When the shapefile is opened with QGIS it seems to have 12 linestrings (shp_qgis.png). However, when the same shapefile is opened with OpenJUMP it contains 85 lines which are different (shp_oj1.png). Ogrinfo does also see 85 linestrings but after running ogr2ogr from shape to shape, the new shapefile is clean and has 12 linestrings (shp_oj2.png). See attached images. Notice that for another software there are some extra features in the shapefile but also some other, newly added with QGIS, which are missing. I would suggest to raise the priority level from high because of corrupted data.
Ogrinfo summary:
ogrinfo TesteLayer.shp -al -so
INFO: Open of TesteLayer.shp' using driverESRI Shapefile' successful.
Layer name: TesteLayer Geometry: Line String Feature Count: 85 Extent: (-54831.944803, 144077.515208) - (-20131.746100, 200497.422392)
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Jukka Rahkonen wrote:
I was reading this stackexchange question http://gis.stackexchange.com/questions/118689/cant-transform-lines-to-polygons with a sample data available for a few days now at http://dropcanvas.com/7i4oq. When the shapefile is opened with QGIS it seems to have 12 linestrings (shp_qgis.png). However, when the same shapefile is opened with OpenJUMP it contains 85 lines which are different (shp_oj1.png). Ogrinfo does also see 85 linestrings but after running ogr2ogr from shape to shape, the new shapefile is clean and has 12 linestrings (shp_oj2.png). See attached images. Notice that for another software there are some extra features in the shapefile but also some other, newly added with QGIS, which are missing. I would suggest to raise the priority level from high because of corrupted data.
it is indeed a strange situation:
another software, gvsig, reads the shape in a different way from qgis and openjump. Re-saving the shape with this sw the first returns a shape with 85 features and the other with 93 (as shown by ogr and qgis).
Any software shows the same data when re-saving the shapefile with qgis or ogr2ogr (12 features).
My guess is that the shape was edited (features deleted) with qgis, this we know now that leave the shape in a state that is inconsistent and gives unexpcted results in other software. If the shape is re-saved after the edits apparently something is used (a parameter in ogr2ogr?) and the vector gets a consistent state.
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
QGIS calls repack() which is supposed to clean the dbf whenever removing/unloading a layer from which features have been removed in the running QGIS session. You can test this behavior by deleting a feature from the test dataset mentioned above and then unloading the layer. The file size of the dbf decreases by a couple of kB.
Calling repack() in a running QGIS session is dangerous because it changes feature ids. And QGIS (and its plugins) should be able to rely on feature ids being unchanged for the lifetime of a layer.
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Matthias Kuhn wrote:
QGIS calls repack() which is supposed to clean the dbf whenever removing/unloading a layer from which features have been removed in the running QGIS session. You can test this behavior by deleting a feature from the test dataset mentioned above and then unloading the layer. The file size of the dbf decreases by a couple of kB.
Calling repack() in a running QGIS session is dangerous because it changes feature ids. And QGIS (and its plugins) should be able to rely on feature ids being unchanged for the lifetime of a layer.
- The easiest solution would be to rewrite the file to another location before sending it to SAGA.
- Or raising a feature request for SAGA to support the dbf flag for deleted features (is this an official feature of this file format?)
- Create an OGR feature request, that repack() can be called without changing feature ids (e.g. it could remap from old feature ids to new feature ids internally. We can't do this on our side, the repack() process is transparent to QGIS I think).
- Let SAGA use a QGIS feature iterator :)
- Decide that feature ids can change in a running session (and internally send a signal that any cached information needs to be discarded)
Hi Matthias, it is not (just) a SAGA issue, that would be the last of our problems.
The issue is also with other very popular gis packages.
Add a shape in qgis, edit it (delete,reshape,split,etc.), save edits. Remove the shape (or not) and open it in such software, the "phantom" features are there... re-save the shape with another name in qgis and everything is ok also in other software. Asking users to re-save shapes before exchanging them seems a bit... strange and unpractical to say the least.
cheers!
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
If you remove features in QGIS, close the project and open the shapefile in other software, the file should be clean. Is it not?
Can you confirm that QGIS cleans the sample data from this report with the steps I outlined above:
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Matthias Kuhn wrote:
If you remove features in QGIS, close the project and open the shapefile in other software, the file should be clean. Is it not?
Can you confirm that QGIS cleans the sample data from this report with the steps I outlined above:
- delete a feature, save
- unload layer
cannot confirm, unloading the layer from project and/or closing qgis does not clean the shape.
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
cannot confirm, unloading the layer from project and/or closing qgis does not clean the shape.
re-saving it does.
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Matthias Kuhn wrote:
If you remove features in QGIS, close the project and open the shapefile in other software, the file should be clean. Is it not?
Can you confirm that QGIS cleans the sample data from this report with the steps I outlined above:
- delete a feature, save
- unload layer
in my pc I can only test open source programs, but what you can see in the attached image happens also with closed source one. The shape was reshaped in qgis, saved, removed from project, closed qgis and opened in openjump.
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
Cannot reproduce this here in this case.
Just to be sure: you deleted a feature first, then you saved, then you removed the layer from the legend, correct (i.e. the deleting part is important, just because you didn't mention it in your comment)?
After doing this here:
INFO: Open of /tmp/orbit-kk/TesteLayer.shp' using driverESRI Shapefile' successful.
Layer name: TesteLayer Geometry: Line String Feature Count: 11 Extent: (-43350.686045, 162465.264117) - (-31426.039433, 174737.909080) Layer SRS WKT: PROJCS["ETRS89_Portugal_TM06", GEOGCS["GCS_ETRS_1989", DATUM["European_Terrestrial_Reference_System_1989", SPHEROID["GRS_1980",6378137,298.257222101]], PRIMEM["Greenwich",0], UNIT["Degree",0.017453292519943295]], PROJECTION["Transverse_Mercator"], PARAMETER["latitude_of_origin",39.66825833333333], PARAMETER["central_meridian",-8.133108333333334], PARAMETER["scale_factor",1], PARAMETER["false_easting",0], PARAMETER["false_northing",0], UNIT["Meter",1]] LAYER: String (32.0) COLOR: Integer (6.0) ID: Integer (5.0)
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Matthias Kuhn wrote:
Cannot reproduce this here in this case.
Just to be sure: you deleted a feature first, then you saved, then you removed the layer from the legend, correct (i.e. the deleting part is important, just because you didn't mention it in your comment)?
notes from the many comments (also in duplicate tickets)
https://issues.qgis.org/attachments/7917/Test501_before_editing.zip
is a good example to replicate the issues.
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
I am using
ogrinfo -al -so
And it always shows the correct feature count. For Test501 it's originally 2 (it's clean in the state when it's downloaded, right?) and subsequently it always reflects the correct number (tried to remove / split / split and remove). I don't know what I can do to try harder... :(
qgib
commented
9 years ago Author Name: Saber Razmjooei (@saberraz)
Matthias,
Try the attached dataset (sample_data.zip). In processing toolbox, use Shape Buffer (SAGA module with 1 metre buffer) and you should get something like edited_vector_buffered_1m.png. Despite the fact that there is no feature on the east side, still it uses the deleted features from the original vector (vector.zip)
Tested in QGIS 2.4 and Master under windows 7 (OSGeo4W install)
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Saber Razmjooei wrote:
Matthias,
Try the attached dataset (sample_data.zip). In processing toolbox, use Shape Buffer (SAGA module with 1 metre buffer) and you should get something like edited_vector_buffered_1m.png. Despite the fact that there is no feature on the east side, still it uses the deleted features from the original vector (vector.zip)
Tested in QGIS 2.4 and Master under windows 7 (OSGeo4W install)
good example Saber.
Matthias, the shapefile that Saber attached shows 4 records (and 4 geometries) when you open in QGIS. But ogrinfo says the features are 9.
Re-save the shape with QGIS and you'll get a copy that ogrinfo will say it has 4 features.
In a similar way I attach the screenshots of 4 gis software that all show the vector with 9 fetaures (9 geometries and 9 records in the attribute table).
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
When I delete a feature in the attached dataset, save the layer and remove it from the project, feature count decreases to 3. I.e. QGIS cleans it up.
Saber, how was this (unclean) dataset produced?
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
That QGIS produces strange results when an unclean dataset is opened is not necessarily a bug in the software, but in the data.
If QGIS originally produced the unclean data, that is something we should worry. But for this we need a reproducible way to create an unclean shapefile.
Right now we only repack() on layer unload if a feature has been deleted in the dataset (that means, that most likely the workflow to corrupt a dataset must not include deleting a feature). We could change the preconditions for a repack(). To identify the proper preconditions we need a reproducible way to create an unclean dataset.
We could also integrate a "corrupted dataset detection" when opening a shapefile that prints a warning and allows to repack() before loading the layer. That would not be a bad idea but if QGIS itself produces unclean datasets right now, this does not solve the root cause.
qgib
commented
9 years ago Author Name: Saber Razmjooei (@saberraz)
??Saber, how was this (unclean) dataset produced???
In QGIS 2.4 Windows 7 (OSGeo4w 64 bit). I will try to see if it is reproducible.
qgib
commented
9 years ago Author Name: Jukka Rahkonen (Jukka Rahkonen)
It is understandable from the QGIS point of view to mark deletions only to .dbf during the edit session. However, delivering such shapefile for other users is not safe. GDAL obviously detects correctly the rows which are marked as deleted in .dbf file and can handle the situation but some other software don't do that. For example OpenJUMP can actually read the geometries from the .shp part even if the .dbf file does not exist at all. OpenJUMP is reading shapefiles with modified GeoTools driver and I suppose that gvSIG is using GeoTools as well which makes me to think that QGIS can make shapefiles which do not work as supposed in GeoServer.
For making it safe it should be guaranteed that 1) Repacking finally happens and 2) Users can't capture edited but not yet repacked shapefiles and pass them on to be used in other software. I guess that 1) is somehow doable but how to implement 2)?
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Hi Matthias,
I just made a pretty basic test:
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Giovanni Manghi wrote:
Hi Matthias,
I just made a pretty basic test:
- opened qgis master
- created from scratch a (polygon) shapefile and added a few features
- saved the edits
- removed the shapefile from project
- added the shapefile to other gis software -> OK
- removed the shapefile from that sw
- added the shapefile to qgis
- edited the shapefile with the reshape tool
- saved edits
- removed the shapefile from qgis
- closed qgis
- added the shapefile to other gis software -> CORRUPTION
not strictly a regression but I really suggest to tag this as blocker and fix/workaround it before 2.6
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
Jukka, it's actually OGR that marks the entries in .dbf as deleted and not QGIS itself. So it's actually not a QGIS issue but an OGR issue and luckily OGR manages to a large degree to handle its own files except for ogrinfo -al -so. For #1 see the next paragraph. For #2 some kind of file locking would need to take place, I don't know if that can be done and if yes, it should probably be done in OGR and not QGIS.
Giovanni, the reshape tool does only change the geometry and not delete a feature. However this somehow still creates deleted entries in the .dbf (not sure what exactly happens there) but I think we could change the preconditions for repack() from "only when features have been deleted" to "always when a layer has been modified".
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
Fixed in changeset "05157f89a06dd65565770303c985a6d0d137ea98".
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Matthias Kuhn wrote:
Fixed in changeset "05157f89a06dd65565770303c985a6d0d137ea98".
Hi Matthias,
this may have introduced a regression when digitizing (adding) a new feature:
now after finishing to digitize a new feature (with right mouse button), the feature does not show until another action is done, like a pan or a zoom.
let me know if you want me to file a new ticket.
cheers!
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Unfortunately I can see that the discussed issue is not solved (but again, re-saving the edited shape "solves" the issue) and that apparently the patch also introduced a regression when adding new features (see above comment).
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
It is very unlikely that the regression you observe and this fix are related. This fix only touches provider code and the feature is not in the provider at the time your regression appears. This is rather introduced by 5e54912565. So, yes, please file a new ticket.
Just to be sure: You did the same test again (on a new shapefile, not an already corrupted one)? I fail to create a corrupted shapefile here, it's pretty hard to isolate the issue like this...
(Tried reshape and subsequent ogrinfo -al -so the Test501.shp from comment #28)
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Matthias Kuhn wrote:
It is very unlikely that the regression you observe and this fix are related. This fix only touches provider code and the feature is not in the provider at the time your regression appears. This is rather introduced by 5e54912565. So, yes, please file a new ticket.
sorry Matthias you are right, there were so many commits yesterday that I lost track of changes while compiling to test your patch.
Just to be sure: You did the same test again (on a new shapefile, not an already corrupted one)? I fail to create a corrupted shapefile here, it's pretty hard to isolate the issue like this...
yes, the test started from a non corrupted shape. I will create a screencast for you, it is very easy to replicate.
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Just to be sure: You did the same test again (on a new shapefile, not an already corrupted one)? I fail to create a corrupted shapefile here, it's pretty hard to isolate the issue like this...
here
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
I guess this
ca be also part of this issue.
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
Ok, I can reproduce it now with OpenJUMP. There seem to be two different problems:
The command ogrinfo -al -so seems to count .dbf entries and I could therefore not detect the corruption with this command. OpenJUMP is more sensitive to this.
The problem is, that even after calling repack, the superfluous geometries in the .shp do not get removed. I am currently out of ideas why that could be.
qgib
commented
9 years ago Author Name: Giovanni Manghi (@gioman)
Matthias Kuhn wrote:
OpenJUMP is more sensitive to this.
basically any other sw tested shows same or similar behavior
The problem is, that even after calling repack, the superfluous geometries in the .shp do not get removed. I am currently out of ideas why that could be.
:(
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
Excerpt from #gdal
TL;DR; OpenJUMP ignores the (updated) .shx, the solution will be that repack also triggers when geometries change. That will be a feature for GDAL/OGR and Even Rouault will be taking care of that.
Please consider buying him a beer at the next FOSS4G.
14:53 < EvenR> well, I know what happens 14:54 < EvenR> when you call SetFeature() in the shapefile driver and that the size of the new geometry blob is larger than the previous size, it puts the new geometry at the end of the shapefile 14:54 < EvenR> and update the index in the .shx accordingly 14:54 < EvenR> perhaps OpenJUMP doesn't read the .shx at all and tries to identify the geometries by scanning the .shp directly 14:55 < mkuhn> sounds like a plausible explanation 14:56 < mkuhn> and like something that will be hard to fix from within QGIS 14:56 < EvenR> that's the way the shapefile driver works 14:57 < EvenR> it could be argued that repack() should also trigger in such situation (rewriting of geometries). Or perhaps a "FORCE REPACK" mode 14:58 < EvenR> repack() basically iterates over all (non-deleted) shapes, writes them in a temporary file, and then rename the temporary file over the normal file 14:58 < mkuhn> the first one would certainly be easier from a qgis developer's perspective ;-) 15:00 < EvenR> mkuhn: I've remove the .shx and openjump can still open the shape (with the warning/error message). So theory confirmed regarding OpenJUMP 15:02 < mkuhn> EvenR: Either it can be blamed on OpenJUMP or the shp repacked under such circumstances, correct? 15:04 < EvenR> mkuhn: OpenJUMP should use the .shx when present certainly. Regarding OGR, if you don't delete records, Repack is currently a no-op as documented ( "Deleted shapes are marked for deletion in the .dbf file, and then ignored by OGR. To actually remove them permanently (resulting in renumbering of FIDs) invoke the SQL 'REPACK...) 15:05 < EvenR> but we could basically make REPACK also trigger each time SetFeature() is called and the new geometry size is != old geometry size 15:06 < mkuhn> In terms of interoperability this would probably be the preferred solution
qgib
commented
9 years ago Author Name: Matthias Kuhn (@m-kuhn)
qgib
commented
9 years ago 
Author Name: Saber Razmjooei (@saberraz) Original Redmine Issue: 11007 Affected QGIS version: 2.8.5 Redmine category:data_provider/ogr Assignee: Marco Hugentobler
When working with SAGA modules, it ignores "Delete flag" in shapefile and uses those deleted features within the processes.
To replicate the issue: 1- Open a shapefile 2- Delete a feature 3- Save the shapefile 4- Run a SAGA process on the shapefile 5- Result of the process takes into account the deleted feature, despite the fact that it has been deleted and saved.
Related issue(s): #17110 (relates), #17515 (relates), #18895 (relates), #18967 (relates), #19680 (relates), #20772 (relates), #21471 (relates), #21478 (duplicates), #21968 (duplicates), #22095 (relates), #22485 (duplicates), #23337 (relates) Redmine related issue(s): 8317, 8822, 10483, 10560, 11398, 12660, 13422, 13431, 13953, 14085, 14512, 15407