Philip Zeyliger has posted comments on this change.

Change subject: IMPALA-5736: Add impala-shell argument to set default query 
options
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8038/1//COMMIT_MSG
Commit Message:

Line 12: Examples:
I think you can add a test in tests/shell/test_shell_interactive.py. 
test_var_substitution() has an example where options are provided. I didn't see 
an ~/.impalarc test.

  @pytest.mark.execute_serially
  def test_var_substitution(self):
    cmds = open(os.path.join(QUERY_FILE_PATH, 
'test_var_substitution.sql')).read()
    args = '''--var=foo=123 --var=BAR=456 --delimited "--output_delimiter= " '''
    result = run_impala_shell_interactive(cmds, shell_args=args)
    assert_var_substitution(result)


PS1, Line 16: .impalarc:
            : [impala]
            : query_options=EXPLAIN_LEVEL=2,MT_DOP=2
This might be hard to do, but I think

[impala]
[[query_options]]
EXPLAIN_LEVEL=2
MT_DOP=2

may be better.


http://gerrit.cloudera.org:8080/#/c/8038/1/shell/option_parser.py
File shell/option_parser.py:

PS1, Line 160:   parser.add_option("--var", dest="keyval", action="append",
             :                     help="Define variable(s) to be used within 
the Impala session."
             :                          " It must follow the pattern 
\"KEY=VALUE\","
             :                          " KEY starts with an alphabetic 
character and"
             :                          " contains alphanumeric characters or 
underscores.")
             :   parser.add_option("--query_options", dest="query_options",
             :                     help="Sets default query options. The format 
is comma separated"
             :   
A little bit of bike-shedding:

Impala-shell seems to support --var a=b --var c=d, but for query options it's 
"--query_options=a=b,c=d". I think the former approach is preferable, because 
it means we don't have to worry about commas in settings. It's also consistent 
with --var.


-- 
To view, visit http://gerrit.cloudera.org:8080/8038
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I26a3b67230c80a99bd246b6af205d558fec9a986
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-HasComments: Yes

Reply via email to