Add BITAP algorithm for search with SQL integration - Githubissues

grapheo12 / iqps

Web-app meant for qp.metakgp.org

MIT License

20 stars 21 forks source link

Add BITAP algorithm for search with SQL integration #36

Closed shubhraneel closed 3 years ago

shubhraneel commented 3 years ago

Written the Bitap function in C. Not yet integrated or tested with MySQL but tested it with words input by the user.

pep8speaks commented 3 years ago

Hello @shubhraneel! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2020-12-12 16:09:58 UTC

shubhraneel commented 3 years ago

This PR if merged will introduce the following changes

The bitap function implemented along with SQL integration
The Makefile for building the db container shall be executed automatically again (if testing the function Bitap we need to run install.sh everytime a change is made to Bitap, including this time)
The bitap.c file previously included is deleted as bitap.cpp in mariadb-udf-docker is used instead
The docker yaml template is changed to automatically start the container for image mariadb-udf instead of mariadb:latest

grapheo12 commented 3 years ago

This is some excellent work! I'll review it tonight.

grapheo12 commented 3 years ago

@shubhraneel Please work on these changes. You are doing great.

shubhraneel commented 3 years ago

I have a doubt. Since I have learned Docker recently, I may be wrong. If I use build: ./mariadb-udf-docker, it builds a new image and then a container but the Makefile in mariadb-udf-docker already creates an image named mariadb-udf so I thought I should use that image created by Makefile itself.

Also, in case you are having a problem where the function does not work, I think deleting the database or using a new folder for the database while running install.sh works.

grapheo12 commented 3 years ago

@shubhraneel I believe the best course of action will be not to build the image in the install.sh script. Just run the makefile locally and commit bitap.so. In this way, you won't require to compile it in user's computer and the build can happen directly. It is better to keep the whole stack in a single docker-compose file. Otherwise all images except the db one will be properly tagged with iqps_ prefix.

grapheo12 commented 3 years ago

One other way is to (and I believe it is better) is to just compile the .so file using the make call in the install.sh file: make bitap.so -C mariadb-udf-docker

shubhraneel commented 3 years ago

I removed the build command from the Makefile. I tested it and it is working. That way the redundant image is not created.

grapheo12 commented 3 years ago

The PR looks good to me! Merging!