-
Notifications
You must be signed in to change notification settings - Fork 3
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
Dataset delete does not work #57
Comments
This is caused by a known bug graphmalizer/graphmalizer-core#18. Graphmalizer's Cypher queries need to be changed, so that these VACANT nodes are deleted. There is an easy fix which would probably also work: Change https://github.com/histograph/neo4j-plugin and make sure it only returns non-VACANT nodes. This can be done here, probably: https://github.com/histograph/neo4j-plugin/blob/master/src/main/java/org/waag/histograph/plugins/ExpandConcepts.java#L33 |
sorry guys, missed this issue! I'll schedule some time to dive into graphmalizer/graphmalizer-core#18 again |
There seems to be a solution for this issue, as suggested by @bertspaan. Issue labeled as ready (to be solved) then. |
VACANT nodes are created on delete? This seems not ok; how to fix? import dataset pdc: |
probably related, didn't have time to look at this, somewhere in |
VACANT: Betekent dit: als je relatie toevoegt tussen A en B, maar PIT’s A en B bestaan (nog) niet, wordt de relatie toegevoegd, en A en B ook, maar A en B zijn dan VACANT There are several issues here, but can we say that adding a dataset and then removing it should leave the repository in an unchanged status? Or is this not true? This can be part of the tests |
Another comment from @wires on Slack: volgens mij is het de situatie |
I only experienced this problem with the faulty rm dataset that had id's like With the VACANT node diagnosis, we should've seen this problem more often and we should be able to reproduce the problem, right? |
From a discussion with @bertspaan:
Number 3 is related to graphmalizer/graphmalizer-core#18 |
Regarding nr 3 above / issue 18 ; the guarantee graphmalizer gives you (when it's not bugged) is that
So they should only appear when there is some edge referring to them. What we notice in #18 is that there appears to arise a situation here a single _VACANT node remains connected to a 'concept node' (the representative node of an equivalence class; the collection of equivalent things). In any case, I can run the tests again, so I will dive into this. ps. It is a little bit subtle as I try to remain some "mathematical" properties of the program. For instance, we -kind of- have the following guarantee on concept nodes:
This can be either useful or annoying or both or neither :-) it also doesn't exclude the case anyway, enough talk! COMPUTEREN! |
This is exactly the case; 100% repeatable by just deleting a dataset. I am now manually (in neo4j) deleting the vacant nodes and relations (of a particular dataset), leaving the concept nodes alone. Tom Demeyer |
Reproducible you say? I'm having trouble with reproducing it... @tomdemeyer @mmmenno could you send me this |
I was reproducing it with a TNL dataset a couple of times. Tom Demeyer
|
If you have a small dataset that does the job, the smaller the better, I’m interested!
Types and schema’s don’t matter; I will turn the dataset into a test
|
I just tested with a small dataset, deletes everything a-ok. except, of course, the ES index… Tom Demeyer
|
@wires you could use (a small subset of) https://api.histograph.io/datasets/rm/pits. To really reproduce this issue, change the id's from |
Someone accidentally uploaded the rm dataset with wrong id's, i think. Suddenly there where id's like rm/8007.0 instead of the rm/8007 it should have been.
Deleting the entire dataset and upload it again with the proper ideas should do the trick, one imagines. Not so. While https://api.histograph.io/datasets/rm/pits gives you a "Dataset 'rm' not found", https://api.histograph.io/search?q=rm/8007.0 keeps returning pits.
Maybe deleting pits / relations from ES doesn't work? Very strange & a bit alarming. Any ideas, @ocataco ?
The text was updated successfully, but these errors were encountered: