GH-43967: [C++] Enhance error message for URI parsing

[CrystalZhou0529]

Rationale for this change

We want to enhance error message for URI parsing error to provide more information for the syntax error scenario.

When error message is generated from uriParseSingleUriExA, the return value might indicate a URI_ERROR_SYNTAX error, and error_pos would be set to the position causing syntax error. (uriparser/Uri.h)

In the new error message, it includes the character causing syntax error and its position, so users can have a better idea why the error happens.

What changes are included in this PR?

• Error message change in URI parsing function.

Are these changes tested?

PR includes unit tests.

Are there any user-facing changes?

Yes, but only for error message.

• GitHub Issue: [Python] Pyarrow fs incorrectly resolves S3 URIs with white space as a local path #41365
• GitHub Issue: [C++] Enhance error message for URI parsing in case of a syntax error #43967

[github-actions]

⚠️ GitHub issue #41365 has been automatically assigned in GitHub to PR creator.

[AlenkaF]

Thank you for working on this fix @CrystalZhou0529 !

From a quick look the change in cpp/src/arrow/util/uri.cc LGTM. I do have some questions/suggestions for the tests.

First: could we also add a test on the Python side to check it works correctly there also?

[pitrou]

This is a neat improvement, thanks @CrystalZhou0529 .

[pitrou]

@AlenkaF

First: could we also add a test on the Python side to check it works correctly there also?

I think the problem is that PyArrow swallows the URI error and falls back to the local filesystem (which is often legitimate, but probably not if the user really meant to pass a S3 URI :-)).

[AlenkaF]

I think the problem is that PyArrow swallows the URI error and falls back to the local filesystem (which is often legitimate, but probably not if the user really meant to pass a S3 URI :-)).

Yeah, the content in the issue should have given me this explanation already. Thanks again ;)

The Java failure is something I noticed on other PRs also if I am not mistaken?

[pitrou]

The Java failure is something I noticed on other PRs also if I am not mistaken?

Yes, it's quite recurring. I'm not sure there's an issue open for it?

[AlenkaF]

I think this might be the open issue connected to the Java failures: #43576

Will merge this PR as the CI is green.

[vibhatha]

@AlenkaF yes this is already tracked and I checked the CIs too. And not relevant to this PR.

[github-actions]

⚠️ GitHub issue #43967 has been automatically assigned in GitHub to PR creator.

[jorisvandenbossche]

First: could we also add a test on the Python side to check it works correctly there also?

I think the problem is that PyArrow swallows the URI error and falls back to the local filesystem (which is often legitimate, but probably not if the user really meant to pass a S3 URI :-)).

Given that this PR is not solving the original issue (I do think we can actually fix that by being more strict in when we swallow that error, and actually bubble it up to the user more often), I opened a separate issue focused on improving the C++ error message and linked this PR to that: #43967

[github-actions]

⚠️ GitHub issue #43967 has been automatically assigned in GitHub to PR creator.

[jorisvandenbossche]

Thanks @CrystalZhou0529!

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 9336bbe.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 22 possible false positives for unstable benchmarks that are known to sometimes produce them.