pyrqlite icon indicating copy to clipboard operation
pyrqlite copied to clipboard

Sort params named_matches by len

Open theraspb3rry opened this issue 11 months ago • 2 comments

This searches for larger (more specific) matches first so that for example gameid and game get matched independently. I found this operated differently from the sqlite3 library, here's some test code.

import pyrqlite.dbapi2 as dbapi2

connection = dbapi2.connect(host="localhost", port=4001)
cursor = connection.cursor()
values = {
    "id": 42069,
    "round": 2,
    "roundname": "Round 2"
}
cursor.execute("CREATE TABLE IF NOT EXISTS foo ( id integer not null primary key, round integer not null, roundname text not null)")

cursor.execute("INSERT INTO foo VALUES(:id, :round, :roundname)", values)

Without the sort, it replaces both round strings first, without giving the chance of replacing roundname to its correct value.

I also added _ to regex to match for params specified as match_id, and from looking at the sqlite docs it seems like table names have a very loose definition for what passes as a table name (with quotes apparently its free game). Not sure how you'd want to handle this one, but from what I remember the regular sqlite3 library did handle the underscores without issue.

Please let me know if you'd like me to modify anything or add a test to catch these in the future. Cheers!

theraspb3rry avatar Mar 24 '24 12:03 theraspb3rry

Actually after doing a little bit more thinking I think it makes more sense to actually refactor this part so we pass the parameters straight to rqlite so we can utilize the existing parametrization api, I assume when this was written rqlite didn't support this yet.

I'll have more of a play to see if I can get this to the point where I can pass all existing tests as long as this makes sense @zmedico ?

theraspb3rry avatar Mar 28 '24 14:03 theraspb3rry

Actually after doing a little bit more thinking I think it makes more sense to actually refactor this part so we pass the parameters straight to rqlite so we can utilize the existing parametrization api, I assume when this was written rqlite didn't support this yet.

Yes, this was introduced in https://github.com/rqlite/rqlite/pull/673 and it would be great to use it.

I'll have more of a play to see if I can get this to the point where I can pass all existing tests as long as this makes sense @zmedico ?

Yes, please do!

zmedico avatar Apr 01 '24 00:04 zmedico