Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-27 Thread Panu Matilainen
Merged #1085 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#event-3077361636___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-27 Thread Panu Matilainen
Thanks for the review!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#issuecomment-591906197___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-27 Thread Michael Schroeder
mlschroe approved this pull request.





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#pullrequestreview-365581861___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Panu Matilainen
Now with a custom match function. Lets see what I managed to mess up this 
time...

/me notes that our test-suite coverage of this stuff is not really adequate for 
all these special cases.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#issuecomment-591418889___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Panu Matilainen
Argh, I totally missed the characters vs bytes distinction.
Maybe it is easiest to just add a custom match afterall :roll_eyes:

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#issuecomment-591390503___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Michael Schroeder
Or: `SUBSTR(key,1,LENGTH('%q')) = '%q'`

But it's also a bit unsafe to assume that `strlen(key) == keylen`. (OTOH the 
prefix iterator is currently not exposed and for rpm's usage the length 
matches).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#issuecomment-591386443___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Michael Schroeder
You could use `INSTR(key, '%q') = 1` but that may be a too slow.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#issuecomment-591368866___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Michael Schroeder
Do we still need that length comparison then?

Also, the sqlite documentation says that for text string the length is UTF8 
characters, not bytes. So this might not work correctly for non-ASCII.

Oh, that's also true for the LENGTH function, it returns characters not bytes. 
I.e. the original code is also incorrect.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#issuecomment-591364610___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Panu Matilainen
@pmatilai pushed 1 commit.

18a2fceea5adc7ba9c71b6e6e7435c33237ef354  Use uppercase for SQL(ite) keywords 
consistently, cosmetics only


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085/files/271529072bdcb132b90201687cd8dc4a77294970..18a2fceea5adc7ba9c71b6e6e7435c33237ef354
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Panu Matilainen
And yeah. I *knew* I was missing something painfully obvious here. Just how 
hard can it be...? :joy:

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085#issuecomment-591361240___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Fix prefix match searches on strings containing % on sqlite backend (#1085)

2020-02-26 Thread Panu Matilainen
SQL LIKE is a quirky thing when you only really want exact matches
on a substring. Which begs the question, if we want exact matches on
substring then why not do just that? Why not, indeed. This avoids
escaping madness and the need to use custom extensions for such
a mundane, trivial thing.

Fixes: #1018
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/1085

-- Commit Summary --

  * Fix prefix match searches on strings containing % on sqlite backend

-- File Changes --

M lib/backend/sqlite.c (4)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/1085.patch
https://github.com/rpm-software-management/rpm/pull/1085.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1085
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint