search

facebookincubator / velox

A composable and fully extensible C++ execution engine library for data management systems.
https://velox-lib.io/
Apache License 2.0
3.54k stars 1.17k forks source link

fix: Match Presto's behavior for invalid UTF-8 in url_decode #11604

Closed kevinwilfong closed 6 days ago

kevinwilfong commented 1 week ago

Summary: Presto Java converts the URL to a Java String after decoding it in url_decode. Java replaces bytes in an invalid UTF-8 character with 0xEF 0xBF 0xBD.

Velox decodes invalid UTF-8 characters as is, which leads to differences in results from Java and C++.

This diff adds a check when decoding URLs for invalid UTF-8 characters and does the same replacement as Java.

Differential Revision: D66249265

netlify[bot] commented 1 week ago

Deploy Preview for meta-velox canceled.

Name Link
Latest commit 5714d63e7c0cbbea9716fc0eacaf12f3fdaaa361
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/673faf7022d4f2000864d2b0
facebook-github-bot commented 1 week ago

This pull request was exported from Phabricator. Differential Revision: D66249265

facebook-github-bot commented 6 days ago

This pull request was exported from Phabricator. Differential Revision: D66249265

facebook-github-bot commented 6 days ago

This pull request was exported from Phabricator. Differential Revision: D66249265

facebook-github-bot commented 6 days ago

This pull request was exported from Phabricator. Differential Revision: D66249265

facebook-github-bot commented 6 days ago

This pull request has been merged in facebookincubator/velox@9ba0197e72d3803ac2ba91f908db6db3f374b5a8.

conbench-facebook[bot] commented 6 days ago

Conbench analyzed the 1 benchmark run on commit 9ba0197e.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.