Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not escaping ' in iLIKE query properly #1038

Closed
mieciu opened this issue Nov 25, 2024 · 1 comment
Closed

Not escaping ' in iLIKE query properly #1038

mieciu opened this issue Nov 25, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@mieciu
Copy link
Member

mieciu commented Nov 25, 2024

I noticed we don't escape ' properly.
I found this out today when messing with filters in eCommerce Revenue Dashboard in OpenSearch.

Screenshot telling more than 1000s of words:

image
@mieciu mieciu added the bug Something isn't working label Nov 25, 2024
@mieciu mieciu changed the title Not escaping ' in `iLIKE query properly Not escaping ' in iLIKE query properly Nov 25, 2024
@trzysiek trzysiek self-assigned this Nov 29, 2024
@trzysiek
Copy link
Member

Fixed with #1114

github-merge-queue bot pushed a commit that referenced this issue Dec 28, 2024
#1038

As new tests show for a little bit, the issue seems fixed. Instead of
doing nothing, I escape all `\` and `'` in all string literals. There
might be some edge case where it's not the correct behaviour, but I
thought for a bit and haven't found any.
Even in `regexp`, where we escape `_` (Clickhouse's equivalent of `.` in
regex), escaping it twice because of this change (sending `\\_` to
Clickhouse instead of `\_` before), seems to work just as fine.

Because of array joins issue, sample panel still doesn't work, but there
are no more any escaping error messages, like in the issue description.
So, when running on `main`, we get a lot of:
```
quesma-1                 | Dec 22 15:36:13.876 ERR quesma/quesma/dual_write_proxy.go:252 > quesma request failed: Q3006: Database query has failed. You may get more details in the Quesma console.  clickhouse: query failed. err: code: 62, message: Syntax error: failed at position 488 ('(') (line 1, col 488): (((arrayJoin("category") __quesma_match 'Men's Clothing')
```
But when running on this branch, we only have errors like this (notice
`'` properly escaped):
```
quesma-1                 | Dec 22 15:32:01.714 ERR quesma/quesma/dual_write_proxy.go:252 > quesma request failed: Q3006: Database query has failed. You may get more details in the Quesma console.  clickhouse: query failed. err: code: 62, message: Syntax error: failed at position 278 ('__quesma_match') (line 1, col 278): __quesma_match 'Men\'s Clothing')
```

**Additional info:**
We discussed with Rafał some other way to make it work: stop sending any
strings in the query itself, but pass all of them as parameters, where
the driver will take care of any escaping issues, etc.
It's probably a good idea for the future, but would require some
non-trivial amount of work, unlike this straightforward solution, which
I already had done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants