Fix KV#list with long (≥50ch) prefixes

[huw]

Resolves #690. See comments for more.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0480e82

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[mrbbot]

Hey! 👋 Great catch! I suspect we might need some workerd changes to fix this, will flag internally. 👍

[huw]

@mrbbot Is there an issue I can follow? I’m not sure if I made it clear but this completely blocks me from using local mode, which means I can’t adopt the Birthday Week features

[huw]

I narrowed this down to https://github.com/cloudflare/workerd/blob/efa6c737e0aa78af29e353cecc88e05537cb44b3/src/workerd/util/sqlite.c%2B%2B#L526, which sets the LIKE clause limit to 50 characters for security reasons. Unless @kentonv is willing to raise it, I think the easier solution is just to avoid LIKE clauses unless we need complex patterns. As such, I updated the PR to run WHERE SUBSTR(key, 1, LENGTH(:prefix)) = :prefix instead and dropped the escaping code. I can’t see many reasons why it would be slower or otherwise undesirable, but you’ll know this better than I will. Thoughts?

[mrbbot]

Hey! 👋 Thank you very much for fixing this, and apologies I didn't get back to you sooner. I asked about this internally, but never followed-up on it. I was going to switch to this exact substr approach if we couldn't get those limits lifted, so very happy with this approach. 😃 Looks like it's still doing an index scan so not concerned about it being slower. 👍

This was also a problem with R2. We're planning to do another patch release today to fix some logging issues. I've committed the required fixes for R2 too, so we can get this in with that. 🙂

[huw]

Goated, cheers mate 🐐