Landmark Routing 1 PR: Landmark database

[vesperlou]

fixes #4134

Issue

Implement a sqlite database as the intermediate storage for parsed landmarks.
Landmark Routing 1: Intermediate Storage for Parsed Landmarks

Parent project

GSOC Landmark-based navigation

[vesperlou]

Just updated new commits to address the code reviews. But the geom column still doesn't work.
I found executing statement for say INSERT INTO landmarks (name, type, geom) VALUES ('Statue of Liberty', 'Monument', CastToPoint(MakePoint(-74.044548, 40.689253, 4326))) doesn't insert data into geom column. The points are always NULL.
Any idea?

[kevinkreiser]

@vesperlou are you doing it inside of a transation? if so you need to commit it every so often so that it actually makes it into the database. transations are a good idea they are a way to batch update the database (shoudl perform better). maybe your class can take have a batch size and then on every batch_size'th item it can commit. the destructor should also commit of course to make sure everything makes it into the database

[vesperlou]

@vesperlou are you doing it inside of a transation? if so you need to commit it every so often so that it actually makes it into the database. transations are a good idea they are a way to batch update the database (shoudl perform better). maybe your class can take have a batch size and then on every batch_size'th item it can commit. the destructor should also commit of course to make sure everything makes it into the database

No I am not using transactions. The current problem is name and type data can store in the database, only points can’t. All points are NULL.
Seems to me I can use transactions but we should fix this first otherwise transactions wouldn’t work :)

[kevinkreiser]

i dont understand how the insert query doesnt fail then. i haven't looked at the code all that closely for that specific part but i assume yuou check the return code about whether it succeeded or failed. to me this looks like it needs a pair programming debug session. alternatively you can just get on the command line msql client and try the queries directly on yoru database.

i would instrument the code to print out each query that is made when it is made and then do them in the same order by starting an sqlite session. you can start the session by running the program sqlite3 in your terminal (you might need to install it)

[kevinkreiser]

oh and also what platform are you on, are you on linux?

[vesperlou]

Yes I’m on linux.
I have checked the documentation which says CastToPoint sometimes will give NULL values to be inserted to the database while the query still succeeds.
Already printed out each query but found nothing. Downloaded a sqlite client to manipulate with my database as well, but didn’t add spatial extension in limit amount of time so functions like MakePoint don’t work yet. Will continue on that.

[vesperlou]

@nilsnolde @kevinkreiser Update: I managed to use DB Browser for SQLite with spatialite extension to check my database and sql statements. It turns out my database is working as expected, the geom column actually stores the correct data, and bounding box query can get the correct answers. But the same things just don't work within the project.

I suspect the problem is adding problematic spatialite extension. In the project I found many functions are not supported such as ST_SetSRID and ST_MakePoint. ST_Point even gets an error saying error: landmarks.geom violates Geometry constraint [geom-type or SRID not allowed]. While in DB Browser all those work properly.

In DB Browser I manually added mod_spatialite by select load_extension('mod_spatialite');. But when I tried to do the same in the project I got Not Authorized error. Not many useful suggestions online.
WDYT?

[vesperlou]

@nilsnolde @kevinkreiser No problem anymore.
I loaded mod_spatialite in the project and solved all issues. But I had to remove make_spatialite_cache which was used to load spatialite as an extension because it stops bounding box query from working.
Feel free to leave any comments or reviews.

[kevinkreiser]

in the next pr where you add parsing real data this will need to be come an enum of type uint8_t

[kevinkreiser]

honestly we should take all this crap and make a pimpl out of it but we can do it some other time. maybe i can pr that in my free time so you can move on to the next task

[kevinkreiser]

i know it seems convenient but it is wasteful for the parser to have to allocate (copy) the parsed data into a new structure just so you can push it into the database. if the builder itself takes a long time this is a place we can look to speed it up. for now we can leave it alone