hdimer commented on PR #2418: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2418#issuecomment-5186666585
Thanks for the thorough review. Pushed a revision: - **No-escape regression:** the escaping is now idempotent. It doubles a lone `]` but leaves an already-doubled `]]` alone, so `[a]]b]` round-trips under `unescape(false)` instead of becoming `[a]]]]b]`. This mirrors what `EscapeQuotedString` already does for `"`. Added a no-escape round-trip test plus a corpus covering leading/trailing/multiple `]`. - **Allocation:** gone. The escaper writes straight into the formatter, no `replace()`. - **Comments:** dropped the issue references. - On testing: I brute-forced round-trips over `]`/letter combinations in both escape modes to check for other failures; the corpus captures the boundaries. One thing worth flagging: like the `"` path, a value that already contains `]]` is treated as already-escaped, so an identifier whose logical value holds two consecutive literal `]` (e.g. default-mode `[a]]]]b]`) now serializes toward the no-escape reading rather than re-doubling. That is the same trade-off `escape_quoted_string` makes for `""`, so bracket and double-quoted identifiers now behave consistently. -- 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]
