webui: Unquote clp queries before submitting them to the backend (fixes #344).

[kirkrodrigues]

References

#344

Description

Users of clp (the "clp" storage engine in the CLP package) expect to be able to quote their queries so that delimiters like spaces are more obvious when sent to the backend. This PR adds support for that.

The specific quoting behaviour is as follows:

• Users can either enter their query without quotes, or with double-quotes (") wrapping both ends of the query.
• Double-quotes within the query must be escaped with the '' escape character.
• Non-double-quotes that are escaped are preserved, e.g., wildcards and notably, escapes.
  • This non-standard behaviour is so that users don't need to insert extra escapes just to pass escaped wildcards to clp.

Validation performed

• Validated new unit tests pass.
• Validated queries against a clp package:
  • Compressed the hive-24hr dataset

    sbin/compress.sh hive-24hr
    

  • Tested the following queries from the webui:
    • info was sent to the backend verbatim
    • " info " was sent as info
    • "\"mapreduce_shuffle\"" was sent as "mapreduce_shuffle"
    • BLOCK\* was sent as BLOCK\*
    • "BLOCK\*" was sent as BLOCK\*
    • \\ was sent as \\
    • "\\" was sent as \\
• Validated queries against clp-s are passed verbatim to the backend.

[haiqi96]

looks good to me. I don't have any specific concern

[junhaoliao]

We can move the empty-string exception throw into unquoteString.

Another thought - generally it is not wrong to have an empty quoted/unquoted string unless we rename/repurpose the function otherwise. Maybe we just need to avoid throwing in a catch.

[kirkrodrigues]

Another thought - generally it is not wrong to have an empty quoted/unquoted string unless we rename/repurpose the function otherwise. Maybe we just need to avoid throwing in a catch.

Yeah, that was my original thought. I'm throwing in the catch just so we can avoid the call again to setOperationErrorMsg, but not a big deal to repeat it.

[junhaoliao]

Another thought - generally it is not wrong to have an empty quoted/unquoted string unless we rename/repurpose the function otherwise. Maybe we just need to avoid throwing in a catch.

Yeah, that was my original thought. I'm throwing in the catch just so we can avoid the call again to setOperationErrorMsg, but not a big deal to repeat it.

Gotcha. I agree with the current handling then.