shivam-startree commented on PR #19453:
URL: https://github.com/apache/pinot/pull/19453#issuecomment-5536202347
Good catch — corrected in `f9921e4`.
Replacing a newline with a space is not semantics-preserving, because a
newline terminates a line comment. Pinot's grammar accepts both forms:
```
<SINGLE_LINE_COMMENT: ("//"|"--")(~["\n","\r"])* ("\n"|"\r"|"\r\n")? >
```
so
```sql
SELECT a -- note
FROM t
```
would have been logged as `SELECT a -- note FROM t` — which no longer
parses, and no longer means what was run. The logged query is something people
copy and replay, so quietly changing it is worse than leaving it on several
lines.
Now escapes CR/LF as `\r` / `\n` instead. Still one physical line, but no
character is removed or reinterpreted, so the record says exactly what was
executed. A query already on one line is returned untouched.
The trade-off is that the logged text needs unescaping before replay. That
seemed clearly better than a form that is silently wrong, but happy to
reconsider if you'd prefer a different rendering.
Also checked for an existing helper before adding one: `LoggerUtils` is the
runtime log-level API, and `commons-text` is managed in the root pom but is not
a `pinot-broker` dependency — and `StringEscapeUtils.escapeJava` would be too
aggressive here, escaping double quotes and unicode and so mangling quoted
identifiers. No existing precedent in the tree for escaping newlines in logged
user text, hence the local two-character replace. Happy to move it to a shared
util if you'd rather.
Added a regression test for the comment case and one asserting single-line
queries pass through unchanged.
--
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]
