valentinsoe commented on PR #7392: URL: https://github.com/apache/datafusion-comet/pull/7392#issuecomment-6638782250
Thanks for checking this against `java.net.URI` directly. You're right about the character gaps. In my run, 340 of the ~40k real page paths (0.85%) were invalid for `java.net.URI`, all because of a `|`. `main` returned NULL for them and this PR returned parsed values. All the other paths matched Spark. I tightened the checks in this PR (81e5b3b2c): - `"`, `\`, `^`, `` ` ``, `|`, DEL and non-ASCII control or space characters are rejected anywhere in the value; - a `%` must be followed by two hex digits; - `[` and `]` are rejected in a hierarchical path, and stay allowed in a query, a fragment or an opaque part, as in `java.net.URI`; - a second `#` is rejected. I also fixed the two cases you found outside this PR, since they sit in the same function: - a scheme name must be a letter followed by letters, digits, `+`, `-` or `.`, and must be followed by a non-empty scheme-specific part, so `http:`, `http:#f` and `user_id:5` are rejected; - a scheme-specific part that does not start with `/` makes the URI opaque, as in `java.net.URI`: `mailto:x#f` gives PROTOCOL `mailto` and REF `f`, and NULL for the other parts. The character checks apply to absolute URLs too, so they also cover the `%zz` and `#a#b` rows of #7330. The registry-based host rows there are still open. With this commit, all of the ~40k real paths, with and without a host prepended, match Spark 4.1.1. On the ~110k generated URLs, no row that matched before stops matching, and the rows that match Spark go from 61k to 96k (out of 110k). The rest differ in the authority (registry-based hosts and IPv6 literals), which is #7330. The new cases are in the `test_parse_url_chars` and `test_parse_url_schemes` tables in `parse_url.sql` and in `parse_url_ansi.sql`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
