from:"Alice Fan \(Code Review\)"

[Impala-ASF-CR] Put transactional tables into 'managed' directory

2020-04-10 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
..


Patch Set 1: Code-Review+1

(1 comment)

Thanks Zoltan for the quick fix. Change looks good to me. Just have one small 
question.

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql@289
PS1, Line 289: CREATE EXTERNAL TABLE IF NOT EXISTS 
{db_name}{db_suffix}.{table_name} (
Just curious, if file format is orc and transnational does not set here. So 
this table is actually created as a managed table?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Fri, 10 Apr 2020 18:17:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9607: Fix test column storage attributes Kudu test

2020-04-07 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15674 )

Change subject: IMPALA-9607: Fix test_column_storage_attributes Kudu test
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b69a342a5b144beceefd17fb5a84547a1ae0103
Gerrit-Change-Number: 15674
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Apr 2020 04:42:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9607: Fix test column storage attributes Kudu test

2020-04-07 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15674 )

Change subject: IMPALA-9607: Fix test_column_storage_attributes Kudu test
..


Patch Set 1: Code-Review+1

Thanks Grant for the quick fix.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b69a342a5b144beceefd17fb5a84547a1ae0103
Gerrit-Change-Number: 15674
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 07 Apr 2020 19:13:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9467: [DOCS] live progress enabled by default in interactive mode

2020-03-16 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15442 )

Change subject: IMPALA-9467: [DOCS] live_progress enabled by default in 
interactive mode
..


Patch Set 2: Code-Review+1

Thanks Kris!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94e624b7bb916ecb5aeb4f007c0610807f7b18cf
Gerrit-Change-Number: 15442
Gerrit-PatchSet: 2
Gerrit-Owner: Kristine Hahn 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 17 Mar 2020 02:50:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9467: [DOCS] live progress enabled by default in interactive mode

2020-03-15 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15442 )

Change subject: IMPALA-9467: [DOCS] live_progress enabled by default in 
interactive mode
..


Patch Set 1:

(4 comments)

Thanks very much for making the doc change. I left some comments to clarify. 
Please feel free to let me know if you have any questions.

http://gerrit.cloudera.org:8080/#/c/15442/1/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/15442/1/docs/shared/impala_common.xml@a1525
PS1, Line 1525:
  :
Just for my curiosity, why do we want to remove the video? I mean It still 
shows the effect of live_progress and live_summary.


http://gerrit.cloudera.org:8080/#/c/15442/1/docs/topics/impala_live_progress.xml
File docs/topics/impala_live_progress.xml:

http://gerrit.cloudera.org:8080/#/c/15442/1/docs/topics/impala_live_progress.xml@a46
PS1, Line 46:
:
:
This paragraph is still true when live_progress = True. We may still want to 
keep it?


http://gerrit.cloudera.org:8080/#/c/15442/1/docs/topics/impala_live_progress.xml@51
PS1, Line 51:  You can turn off LIVE_PROGRESS within the 
shell
:   using the SET command. 
IMPALA-9384 also proposed a new command line flag "--disable_live_progress". 
The flag allows users to disable live_progress in  the runtime. e.g. run impala 
shell with "bin/impala-shell.sh --disable_live_progress" then live_progress 
will be disable in the interactive mode.

In addition, setting live_progress = false in the config file is another way to 
disable the shell option.


http://gerrit.cloudera.org:8080/#/c/15442/1/docs/topics/impala_shell_options.xml
File docs/topics/impala_shell_options.xml:

http://gerrit.cloudera.org:8080/#/c/15442/1/docs/topics/impala_shell_options.xml@a532
PS1, Line 532:
 :
 :
 :
 :
 :
 :
we still want to keep this right? as --live_progress is still an impala shell 
option. We may also want to add --disable_live_progress.

// for --live_progress

  --live_progress
  live_progress=true
  Prints a progress bar .


// for --disable_live_progress

  --disable_live_progress
  live_progress=false
  A command line flag allows users to disable live_progress in the 
interactive mode.




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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94e624b7bb916ecb5aeb4f007c0610807f7b18cf
Gerrit-Change-Number: 15442
Gerrit-PatchSet: 1
Gerrit-Owner: Kristine Hahn 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 16 Mar 2020 01:27:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-05 Thread Alice Fan (Code Review)

Hello Andrew Sherman, David Knupp, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15219

to look at the new patch set (#8).

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..

IMPALA-9384: Improve Impala shell usability by enabling live_progress in 
interactive mode

In order to improve usability, this patch makes Impala shell show query
processing status while the query is running. The patch enables shell
option live_progress by default when a user launches impala shell in
the interactive mode. The patch also adds a new command line flag
"--disable_live_progress", which allows a user to disable live_progress
at runtime. In the interactive mode, a user can disable live_progress
by either using the command line flag or setting the option as False in
the config file. As for in the non-interactive mode (when the -q or -f
options are used), live reporting is not supported. Impala-shell will
disable live_progress if the mode is detected.

Testing:
- Added and updated tests in test_shell_interactive.py and 
test_shell_commandline.py
- Successfully ran all shell related tests

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 47 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/8
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-04 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..


Patch Set 6:

(1 comment)

Patch 6 includes the changes. Please ignore patch 4 and 5. Tried resubmit to 
resolve wrap line warnings.

http://gerrit.cloudera.org:8080/#/c/15219/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/15219/3/shell/option_parser.py@307
PS3, Line 307: # print default value of disable_live_progress in the help 
messages as opposite
> Ah, you're totally right. Sorry -- I read too quickly, and didn't notice th
Thanks. Have added the check for mutually exclusive options.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Mar 2020 00:55:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-04 Thread Alice Fan (Code Review)

Hello Andrew Sherman, David Knupp, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15219

to look at the new patch set (#6).

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..

IMPALA-9384: Improve Impala shell usability by enabling live_progress in 
interactive mode

In order to improve usability, this patch makes Impala shell show query
processing status while the query is running. The patch enables shell
option live_progress by default when a user launches impala shell in
the interactive mode. The patch also adds a new command line flag
"--disable_live_progress", which allows a user to disable live_progress
at runtime. In the interactive mode, a user can disable live_progress
by either using the command line flag or setting the option as False in
the config file. As for in the non-interactive mode (when the -q or -f
options are used), live reporting is not supported. Impala-shell will
disable live_progress if the mode is detected.

Testing:
- Added and updated tests in test_shell_interactive.py and 
test_shell_commandline.py
- Successfully ran all shell related tests

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 46 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/6
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-04 Thread Alice Fan (Code Review)

Hello Andrew Sherman, David Knupp, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15219

to look at the new patch set (#5).

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..

IMPALA-9384: Improve Impala shell usability by enabling live_progress in 
interactive mode

In order to improve usability, this patch makes Impala shell show query
processing status while the query is running. The patch enables shell
option live_progress by default when a user launches impala shell in
the interactive mode. The patch also adds a new command line flag
"--disable_live_progress", which allows a user to disable live_progress
at runtime. In the interactive mode, a user can disable live_progress
by either using the command line flag or setting the option as False in
the config file. As for in the non-interactive mode (when the -q or -f
options are used), live reporting is not supported. Impala-shell will
disable live_progress if the mode is detected.

Testing:
- Added and updated tests in test_shell_interactive.py and 
test_shell_commandline.py
- Successfully ran all shell related tests

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 46 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/5
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-04 Thread Alice Fan (Code Review)

Hello Andrew Sherman, David Knupp, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15219

to look at the new patch set (#4).

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..

IMPALA-9384: Improve Impala shell usability by enabling live_progress in 
interactive mode

In order to improve usability, this patch makes Impala shell show query
processing status while the query is running. The patch enables shell
option live_progress by default when a user launches impala shell in
the interactive mode. The patch also adds a new command line flag
"--disable_live_progress", which allows a user to disable live_progress
at runtime. In the interactive mode, a user can disable live_progress
by either using the command line flag or setting the option as False in
the config file. As for in the non-interactive mode (when the -q or -f
options are used), live reporting is not supported. Impala-shell will
disable live_progress if the mode is detected.

Testing:
- Added and updated tests in test_shell_interactive.py and 
test_shell_commandline.py
- Successfully ran all shell related tests

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 45 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/4
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-03 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..


Patch Set 3:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/15219/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15219/3//COMMIT_MSG@9
PS3, Line 9: In order to improve usability, this patch would like to make 
Impala shell show query
> The commit message describes the change. So "this patch makes Impala Shell
Thanks. will update.


http://gerrit.cloudera.org:8080/#/c/15219/3//COMMIT_MSG@11
PS3, Line 11: live_progress by default when a user launches impala shell in the 
interactive mode.
> Nit: in general, unless you have some special text, you should word wrap at
Sure. will do. Thanks


http://gerrit.cloudera.org:8080/#/c/15219/3//COMMIT_MSG@14
PS3, Line 14: live_progress by either using the proposed command line flag or 
setting the option as
> Don't say 'proposed' the commit message describes what changed.
Thanks. will remove 'proposed'


http://gerrit.cloudera.org:8080/#/c/15219/3/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/15219/3/shell/impala_shell.py@1845
PS3, Line 1845: options.live_progress = False
> It might be nice to issue a message to the console here indicating that thi
Thanks, will add warning message here.


http://gerrit.cloudera.org:8080/#/c/15219/3/shell/impala_shell.py@1847
PS3, Line 1847:   print_to_stderr("Error: Live reporting is available for 
interactive mode only.")
> Of course, now this makes me wonder whether we should just set live_summary
sure. will do


http://gerrit.cloudera.org:8080/#/c/15219/3/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/15219/3/shell/impala_shell_config_defaults.py@42
PS3, Line 42: 'live_progress': True,
> Nit: you could add a comment above or beside here, indicating that this onl
sure. will do


http://gerrit.cloudera.org:8080/#/c/15219/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/15219/3/shell/option_parser.py@238
PS3, Line 238:  " Users can disable it by setting it as 
False in config file.")
> I think that last sentence might be more clear like this -- what do you thi
Thanks. will update this


http://gerrit.cloudera.org:8080/#/c/15219/3/shell/option_parser.py@307
PS3, Line 307: elif option == parser.get_option('--disable_live_progress'):
> Technically, I don't think this is necessary.
I think the reason I added this line is because we want to make the help text's 
default value of --disable_live_progress is False

For example: run impala-shell.sh --help

--disable_live_progress
   A command line flag allows users to disable
   live_progress in the interactive mode. [default:
   False] // This need to be False and which is opposite to 
--live_progress = True


Because --disable_live_progress and --live_progress shared the same 
dest="live_progress".
The help text will print the default value of the destination flag, and we need 
to make sure it will print the opposite value.


As for the two mutually exclusive flags are used case. I think the last one 
always take precedence.
Ex: If a user run impala-shell.sh --verbose --quiet
Impala shell will set verbose = false.
This looks like be a consistent behavior and may not need a error to handle it?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 3
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 03 Mar 2020 22:19:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-02 Thread Alice Fan (Code Review)

Hello Andrew Sherman, David Knupp, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15219

to look at the new patch set (#3).

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..

IMPALA-9384: Improve Impala shell usability by enabling live_progress in 
interactive mode

In order to improve usability, this patch would like to make Impala shell show 
query
processing status while the query is running. The patch enables shell option
live_progress by default when a user launches impala shell in the interactive 
mode.
The patch also provides a new command line flag "--disable_live_progress", 
which allows
a user to disable live_progress at runtime. In the interactive mode, a user can 
disable
live_progress by either using the proposed command line flag or setting the 
option as
False in the config file. As for in the non-interactive mode (when the -q or -f 
options
are used), live reporting is not supported. Impala-shell will disable 
live_progress if
the mode is detected.

Testing:
- Added and updated tests in test_shell_interactive.py and 
test_shell_commandline.py
- Successfully ran all shell related tests is detected.

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 33 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/3
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 3
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by enabling live progress in interactive mode

2020-03-02 Thread Alice Fan (Code Review)

Hello Andrew Sherman, David Knupp, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15219

to look at the new patch set (#2).

Change subject: IMPALA-9384: Improve Impala shell usability by enabling 
live_progress in interactive mode
..

IMPALA-9384: Improve Impala shell usability by enabling live_progress in 
interactive mode

In order to improve usability, this patch would like to make Impala shell show 
query
processing status by default. The patch enables shell option live_progress when 
a user
launches impala shell in the interactive mode. The patch also provides a new 
command
line flag “--disable_live_progress”, which allows a user to disable 
live_progress at
runtime. In the interactive mode, a user can disable live_progress by either 
using
the proposed command line flag or setting the option as False in the config 
file. As for
in the non-interactive mode (when the -q or -f options are used), live 
reporting is not
supported. Impala-shell will disable live_progress if the mode is detected.

Testing:
- Added and updated tests in test_shell_interactive.py and 
test_shell_commandline.py
- Successfully ran all shell related tests

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
M tests/shell/test_shell_interactive.py
5 files changed, 33 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/2
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by automatically enable live progress in the interactive mode

2020-02-28 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by automatically 
enable live_progress in the interactive mode
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py@1742
PS1, Line 1742:   config_file_live_progress = s_options.get('live_progress')
> Line 1731 set config_file_live_progress = None
Thanks for catching this. I left a comment for option 1 and 2 according to 
David's feedback. Let's see which path we decide to go.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 28 Feb 2020 23:33:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by automatically enable live progress in the interactive mode

2020-02-28 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by automatically 
enable live_progress in the interactive mode
..


Patch Set 1:

Thanks David for your input. I would like to summarize the two possible options 
here:

Option 1: current patch 1 + modification for --disable_live_progress
Won’t set live_progress = true in impala_shell_config_default.py
In impala_shell.py, we set live_progress to True when users neither set it in 
configuration file nor in command line AND it is in the interactive mode.

Option 2: set live_progress to TRUE in impala_shell_config_default.py
set live_progress to true in impala_shell_config_default.py
Impala_shell implicitly disables live_progress when the non-interactive mode is 
detected even user set it to true in config file or/and command line
Remove the check and throw FatalShellException for the case of detecting 
live_progress is true in non-interactive mode because of (2)

Either choosing option 1 or 2, we can add “--disable_live_progress” This allows 
users to explicitly disable live_progress by command line when they are in 
interactive mode.

To me, I think option 2 looks more straightforward. That is - in the 
non-interactive mode, live_progress will always be false no matter if a user 
tries to set it to true in config file and/or command line. Impala_shell will 
implicitly set it to false when detecting it is in non-interactive mode. As for 
interactive mode, by default live_progress is set to True, a user has options 
to set it to false in either command line or config file.

David and Andrew, please let me know if you agree with option 2? Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 28 Feb 2020 23:17:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by automatically enable live progress in the interactive mode

2020-02-28 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by automatically 
enable live_progress in the interactive mode
..


Patch Set 1:

Hi David, Thanks for your feedback. As we discussed offline, please let me know 
when you have an update about how to proceed. Thanks


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 28 Feb 2020 19:05:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by automatically enable live progress in the interactive mode

2020-02-28 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15219 )

Change subject: IMPALA-9384: Improve Impala shell usability by automatically 
enable live_progress in the interactive mode
..


Patch Set 1:

(8 comments)

Thanks for the feedback Andrew.

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

http://gerrit.cloudera.org:8080/#/c/15219/1//COMMIT_MSG@8
PS1, Line 8: live_progress in the interactive mode
> I think the single line starting with 'IMPALA-9384' should be a short singl
sure. will change.


http://gerrit.cloudera.org:8080/#/c/15219/1//COMMIT_MSG@12
PS1, Line 12: mode when users do not set the option in both configure file and 
command
> Should this be "in either the configuration file or as a command line flag"
sure. will update.


http://gerrit.cloudera.org:8080/#/c/15219/1//COMMIT_MSG@19
PS1, Line 19:
> Add a documentation jira to update docs for the change, or refer to it in t
sure. will file a documentation jira for it.


http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py@1728
PS1, Line 1728:   # config_file_live_progress is None, which means user didn't 
set live_progress
> Nit: maybe clearer to say "Set config_file_live_progress to None" ...
sure. will change


http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py@1742
PS1, Line 1742:   config_file_live_progress = s_options.get('live_progress')
> As this is being set in a loop, is it possible that the value can be set to
I think I didn't get what you mean here. can you please elaborate? thanks


http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py@1753
PS1, Line 1753:   # live_progress in the config file, Impala shell will 
automatically enable it
> Nit: end with a period
thanks. will do


http://gerrit.cloudera.org:8080/#/c/15219/1/shell/impala_shell.py@1754
PS1, Line 1754:   if not options.query and not options.query_file and 
config_file_live_progress is None:
> Can we simplify by using 'self.interactive' here?
I think we are doing the if statement at main() and the self.interactive is 
defined as private variable of ImpalaShell class. So here we may have to do the 
same like 
https://github.com/apache/impala/blob/master/shell/impala_shell.py#L1842


http://gerrit.cloudera.org:8080/#/c/15219/1/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/15219/1/tests/shell/test_shell_interactive.py@503
PS1, Line 503: # file and command line, Impala shell will automatically 
enable it
> Nit: end comment with a period
Thanks. will change.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 28 Feb 2020 18:40:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7031: Improve error message for invalid query handle

2020-02-13 Thread Alice Fan (Code Review)

Alice Fan has abandoned this change. ( http://gerrit.cloudera.org:8080/12926 )

Change subject: IMPALA-7031: Improve error message for invalid query handle
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 12
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-9384: Improve Impala shell usability by automatically enable live progress in the interactive mode

2020-02-13 Thread Alice Fan (Code Review)

Alice Fan has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15219


Change subject: IMPALA-9384: Improve Impala shell usability by automatically 
enable live_progress in the interactive mode
..

IMPALA-9384: Improve Impala shell usability by automatically enable
live_progress in the interactive mode

Currently, shell option live_progress is set to False by default.
The patch makes Impala shell automatically enable it in the interactive
mode when users do not set the option in both configure file and command
line. This will help to improve usability by automatically showing the
query processing status to users.

Testing:
- Updated and added tests in test_shell_interactive.py
- Successfully ran all other shell interactive related tests

Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 15 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/15219/1
--
To view, visit http://gerrit.cloudera.org:8080/15219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3765b775f663fa227e59728acffe4d5ea9a5e2d3
Gerrit-Change-Number: 15219
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

2019-11-05 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14540 )

Change subject: IMPALA-9023: Fix IllegalStateException in 
SimplifyConditionalsRule
..


Patch Set 4:

My functional.alltypes data is messed up, so one of the newly added query tests 
will return incorrect result. I have fixed it. Sorry about that.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
Gerrit-Change-Number: 14540
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 06 Nov 2019 03:13:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

2019-11-05 Thread Alice Fan (Code Review)

Hello Thomas Tauber-Marshall, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/14540

to look at the new patch set (#4).

Change subject: IMPALA-9023: Fix IllegalStateException in 
SimplifyConditionalsRule
..

IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

For 'case' function, if left WHEN condition is true, SimplifyConditionalsRule
will cast the THEN result-expression to original expression's type before return
result. In this Jira, we would like to remove the cast function for two reasons:
1. SimplifyConditionalsRule only applys to analyzed expression, which means
expression has already been casted to compatible type before it reaches the
expression rewrite step.
2. The cast function will cause IllegalStateException when 'CASE WHEN TRUE'
appearing in the where conjunction. For example:
Query: select * from functional.alltypessmall where case when true then id < 50 
END
ERROR: IllegalStateException: null

Testing:
- Added e2e test to exprs.test
- Added unit test to ExprRewriteRulesTest
- Added unit test to ExprRewriterTest

Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
---
M fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 54 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/14540/4
--
To view, visit http://gerrit.cloudera.org:8080/14540
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
Gerrit-Change-Number: 14540
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

2019-10-31 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14540 )

Change subject: IMPALA-9023: Fix IllegalStateException in 
SimplifyConditionalsRule
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14540/1/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java:

http://gerrit.cloudera.org:8080/#/c/14540/1/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@253
PS1, Line 253: return expr.getChild(i + 1);
> I don't think this is necessarily the right fix since the rewrite should no
As we discussed offline. It should be safe to remove cast here. As the 
SimplifyConditionalsRule only applies to analyzed expression , so the expr has 
ready been casted before it reaches rewrite step. Also, for some reason, the 
cast function will make a part of conjunction become not analyzed result in 
llegalStateException.


http://gerrit.cloudera.org:8080/#/c/14540/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:

http://gerrit.cloudera.org:8080/#/c/14540/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@502
PS1, Line 502: + "END", rule, "id > 30");
> Can you add some expression rewriter test where the two branches have diffe
I added some test cases in ExprRewriterTest. But I think we knew that the 
change won't fix the issue of casting to compatible type for rewrite. That will 
require a separate jira to address the re-analyze issue. Really appreciated 
your examples and explanation!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
Gerrit-Change-Number: 14540
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 01 Nov 2019 02:32:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

2019-10-31 Thread Alice Fan (Code Review)

Hello Thomas Tauber-Marshall, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/14540

to look at the new patch set (#2).

Change subject: IMPALA-9023: Fix IllegalStateException in 
SimplifyConditionalsRule
..

IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

For 'case' function, if left WHEN condition is true, SimplifyConditionalsRule
will cast the THEN result-expression to original expression's type before return
result. In this Jira, we would like to remove the cast function for two reasons:
1. SimplifyConditionalsRule only applys to analyzed expression, which means
expression has already been casted to compatible type before it reaches the
expression rewrite step.
2. The cast function will cause IllegalStateException when 'CASE WHEN TRUE'
appearing in the where conjunction. For example:
Query: select * from functional.alltypessmall where case when true then id < 50 
END
ERROR: IllegalStateException: null

Testing:
- Added e2e test to exprs.test
- Added unit test to ExprRewriteRulesTest
- Added unit test to ExprRewriterTest

Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
---
M fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 54 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/14540/2
--
To view, visit http://gerrit.cloudera.org:8080/14540
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
Gerrit-Change-Number: 14540
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

2019-10-24 Thread Alice Fan (Code Review)

Alice Fan has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/14540


Change subject: IMPALA-9023: Fix IllegalStateException in 
SimplifyConditionalsRule
..

IMPALA-9023: Fix IllegalStateException in SimplifyConditionalsRule

For 'case' function, if left WHEN condition is true, SimplifyConditionalsRule
will return THEN result-expression. However, the result-expression should not
be casted to original expression's type. This will cause result-expression
not re-analyzed and throwing IllegalStateException.

For example:
Query: select * from functional.alltypessmall where case when true then id < 50 
END
ERROR: IllegalStateException: null

Testing:
- Added e2e test to exprs.test
- Added unit test to ExprRewriteRulesTest

Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
---
M fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
3 files changed, 21 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/14540/1
--
To view, visit http://gerrit.cloudera.org:8080/14540
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I640d577200e76121c72685e4aaba1ef312a2d8b4
Gerrit-Change-Number: 14540
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-14 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..


Patch Set 11:

Checked failed tests in verified build.
Fixed the relevant test 'AnalyzeExprsTest.TestDecimalCasts' by converting 
expression of error message to all upper case.

'metadata.test_compute_stats.TestComputeStats.test_compute_stats_compression_codec'
 and 
'metadata.test_refresh_partition.TestRefreshPartition.test_refresh_partition_num_rows'
 are irrelevant to the change. Tested the patch 11 at private-parameterized 
build and these two tests are both passed.

Link to patch 11's private-parameterized build: 
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5035/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Tue, 14 May 2019 17:54:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-14 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#11).

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
7 files changed, 86 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/11
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7031: Improve error message for invalid query handle

2019-05-10 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#12).

Change subject: IMPALA-7031: Improve error message for invalid query handle
..

IMPALA-7031: Improve error message for invalid query handle

The patch added additional information to error message of invalid query.
If a client tries to use the query handle of a recently unregistered query.
Except "Invalid query handle", error message should also indicate that the
query is already unregistered.

Testing:
- Added a test to test_cancellation.py

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/query_test/test_cancellation.py
5 files changed, 52 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/12
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 12
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-09 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12926 )

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..


Patch Set 11:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12926/10/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/12926/10/tests/hs2/test_hs2.py@336
PS10, Line 336: assert err_msg in get_operation_status_resp.status.errorMessage
> wont this fail if the error message is "Invalid query handle. The query is
Thanks for catching this. This will not be put into archive log


http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py@549
PS10, Line 549: """Verify that an indicative error message is returned when 
trying to use a query
> nit: Verify that
Done


http://gerrit.cloudera.org:8080/#/c/12926/10/tests/webserver/test_web_pages.py@551
PS10, Line 551: is still in the query archive log, so to make sure it is 
not pushed out of the log by
> nit: out of the log
Thanks.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 10 May 2019 00:25:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-09 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#11).

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..

IMPALA-7031: Add additional info to query canceled from http endpoint

When a running query is canceled from the http endpoint, the query will
be unregistered. This can result in a scenario where a client that has
a reference to the query handle is unaware that the query has been
unregistered via http endpoint, and attempts to run an operation on
that query handle but encounters "Invalid query handle" error.
>From the user prospective the error is not indicative, therefore the
patch added additional information to the error to notify that the query
is already unregistered.

Testing:
- Added a test to test_web_pages.py
- Ran all webserver tests

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/webserver/test_web_pages.py
5 files changed, 55 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/11
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 11
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-09 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13050/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/13050/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2327
PS8, Line 2327:* widest compatible expression encountered among all i-th 
exprs in the expr lists.
> nit: lists
among all i-th exprs in the expr list(S).


http://gerrit.cloudera.org:8080/#/c/13050/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2328
PS8, Line 2328:* Returns null if an empty expression list or null is passed 
to it.
> Returns null if an empty expression list or null is passed to it.
Thanks. Added it



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Fri, 10 May 2019 00:05:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-09 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#9).

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 85 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/9
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-08 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS7, Line 2326: Returns a list of exprs such that for every i-th expr in that 
list, it is the first
  :* widest compatible expression encountered among all
> sorry for changing the suggestions for the comments again and again, the te
Sure. Thank you!


http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2332
PS7, Line 2332: if (exprLists == null || exprLists.size() == 0) return null;
> I think having an if statement for exprLists.size() == 1 wont hurt. Just re
Done


http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/7/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@161
PS7, Line 161:   // Contains a list of exprs such that for every i-th expr in 
that list, it is the first
 :   // widest compatible expressio
> Contains a list of exprs such that for every i-th expr in that list, it is
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Thu, 09 May 2019 05:51:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-08 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#8).

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 84 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/8
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-08 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12926 )

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..


Patch Set 9:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:

http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-beeswax-server.cc@201
PS9, Line 201: ImpalaServer::
> nit: you can just call the method without this scope resolution.
Done


http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h@605
PS9, Line 605:   // When a query is unregistered, its QueryStateRecord will be 
added into query log
 :   // during the archive stage. This method is used to identify 
whether the query is
 :   // unregistered and return a proper error message. The method 
is used for unknown
 :   // query, which is a query lost its ClientRequestState.
> Returns the appropriate error message for an invalid query id. Also checks
Done


http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.h@609
PS9, Line 609: getErrMsgForUnknownQuery
> nit: how about getErrMsgForInvalidQueryId
sure.


http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/12926/9/be/src/service/impala-server.cc@2589
PS9, Line 2589: Query is already unregistered
> I think we should still say invalid query handle in this case too, because
Done


http://gerrit.cloudera.org:8080/#/c/12926/9/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/12926/9/tests/hs2/test_hs2.py@335
PS9, Line 335:
 : assert "Query is already unregistered" or "Invalid query 
handle" \
 : in get_operation_status_resp.status.errorMessage
> ("a" or "b" in str) is the wrong conditional, please see the following link
Thanks!
Removed changes here. As error msg now will include 'Invalid query handle' in 
anyway.


http://gerrit.cloudera.org:8080/#/c/12926/9/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:

http://gerrit.cloudera.org:8080/#/c/12926/9/tests/query_test/test_cancellation.py@194
PS9, Line 194: ('Invalid query handle' in str(thread.fetch_results_error) or
 :  'Query is already unregistered' in 
str(thread.fetch_results_error)
 :  and not join_before_close)
> this becomes A || B & C, which is interpreted as A || (B & C)
Woops. Thanks for point this out!
I removed the "Query is already unregistered" from here, as it is an extra info 
for "Invalid query handle"


http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@546
PS9, Line 546:
> add @pytest.mark.execute_serially decorator to this test. You should make s
Thanks!


http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@548
PS9, Line 548: """When a query is successfully canceled, client should be 
notified that the
 : query is already unregistered."""
> nit:how about: Make sure an indicative error message is returned when tryin
Done


http://gerrit.cloudera.org:8080/#/c/12926/9/tests/webserver/test_web_pages.py@562
PS9, Line 562:   # Because the query is unregistered, client should receive 
exception with
 :   # "Query is already unregistered" or "Invalid query 
handle" notice.
 :   # For the query gets "Invalid query handle" is because 
query log might be
 :   # full and then removed the oldest unregistered query
> nit: you can remove this comment after you make the change to run this seri
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 09 May 2019 03:16:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-08 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#10).

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..

IMPALA-7031: Add additional info to query canceled from http endpoint

When a running query is canceled from the http endpoint, the query will
be unregistered. This can result in a scenario where a client that has
a reference to the query handle is unaware that the query has been
unregistered via http endpoint, and attempts to run an operation on
that query handle but encounters "Invalid query handle" error.
>From the user prospective the error is not indicative, therefore the
patch added additional information to the error to notify that the query
is already unregistered.

Testing:
- Added a test to test_web_pages.py
- Ran all webserver tests

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/webserver/test_web_pages.py
5 files changed, 55 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/10
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 10
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-08 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#7).

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 82 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/7
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-08 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2332
PS6, Line 2332: exprLists.size() < 2
> looks like if exprLists.size() =1 then it should return exprLists.get(0)
Thanks for catching this!
Originally, I thought the exprLists.size() == 1 case will be handle by 
statementBase.checkTypeCompatibility(). widestExpr == null, basically will be 
replaced by srcExpr, which is the exprLists.get(0).


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2348
PS6, Line 2348: if (preType != compatibleType) {
  :   widestExprs.set(i, exprLists.get(j).get(i));
  : }
> nit: might fit in one line
Done


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@699
PS6, Line 699:   Expr widestTypeExpr = null;
 :   if (widestTypeExprList != null) {
 : widestTypeExpr = widestTypeExprList.get(i);
 :   }
> nit: can be one line
Done


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@55
PS6, Line 55:   // Widest (highest precision) expression is a list of the first 
widest compatible
:   // expression for each column. The list is stored in the order 
of target columns.
:   protected List widestExprs_ = new ArrayList<>();
> this needs to go after L133 ( in the list of members that need to be reset(
Moved into the list of "members that need to be reset()".

Remove the part for stored in order, because there are other list members 
doesn't not explain the order. Also, the order is explained at comment area of 
analyzer.castToUnionCompatibleTypes() when the widestExprs is returned.

reset() method has been updated at previous patch. which is at line648. also 
update clone at line 199


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:

http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS6, Line 3404: on
> nit: typo: blame the widest
Updated comments.


http://gerrit.cloudera.org:8080/#/c/13050/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3407
PS6, Line 3407:
> can you add a one line comment for each test case as to what it is testing.
added short comment for each of test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 08 May 2019 22:18:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-07 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..


Patch Set 5:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/13050/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/5//COMMIT_MSG@7
PS5, Line 7: IMPALA-966: Type errors are attributed to wrong expression with 
insert
   :
   : When insert multiple incompatible type values into a table,
   : error message should blame on the correct expression. If there
   : are multiple incompatible type values for a single target
   : column, error should blame on the first widest incompatible type
   : expression.
> how about :
Nice example. Much appreciate for the help!


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@692
PS5, Line 692:   // If the queryStmt_ is a unionStmt, it will return a 
WidestExprs list
 :   // when do castToUnionCompatibleTypes().
 :   // widestTypeExpr will be null if the queryStmt_ is a 
SelectStmt
> nit: superfluous comment
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@695
PS5, Line 695:   UnionStmt unionStmt =
 :   (queryStmt_ instanceof UnionStmt) ? (UnionStmt) 
queryStmt_ : null;
 :   if (unionStmt != null && unionStmt.getWidestExprs() != null
 :   && unionStmt.getWidestExprs().size() > 0) {
 : widestTypeExpr = unionStmt.getWidestExprs().get(i);
 :   }
> nit: instead of doing this in every loop maybe just get the widestExprList
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
File fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java@292
PS5, Line 292: null
> nit: remove the comment above and add an inline comment here like
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS5, Line 196: for
> nit: among
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS5, Line 196: widestTypeSrcExpr
> nit: add quotes since this refers to an input param
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@197
PS5, Line 197: Error message should blame on the widestTypeSrcExpr instead of 
the first
 :* compatible source expression.
> nit: is only used when constructing an AnalysisException message to make su
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@56
PS5, Line 56:   // widestExprs_ is a list of the first widest compatible 
expression for each column
> nit: you can remove the first line  and write "widest (highest precision)"
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:

http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS5, Line 3404: on
> nit: the
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3404
PS5, Line 3404:  // Error should blame on correct expression.
  : // The widest (highest precision) expression and type 
should appear in error.
> nit: these two are a bit repetitive. can you consolidate both into one?
Done


http://gerrit.cloudera.org:8080/#/c/13050/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3422
PS5, Line 3422:   }
> can you add a case where there are multiple expressions with the same incom
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice

[Impala-ASF-CR] IMPALA-966: Attribute type error to the right expression in an insert statement

2019-05-07 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#6).

Change subject: IMPALA-966: Attribute type error to the right expression in an 
insert statement
..

IMPALA-966: Attribute type error to the right expression in an insert
statement

Currently, if an insert statement contains multiple expressions
that are incompatible with the column type, the error message
returned attributes the error to the wrong expression.
This patch makes sure the right expression is blamed. If there are
multiple incompatible type values for the target column, then the
error is attributed to the first widest (highest precision)
incompatible type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 78 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/6
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

2019-05-04 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#5).

Change subject: IMPALA-966: Type errors are attributed to wrong expression with 
insert
..

IMPALA-966: Type errors are attributed to wrong expression with insert

When insert multiple incompatible type values into a table,
error message should blame on the correct expression. If there
are multiple incompatible type values for a single target
column, error should blame on the first widest incompatible type
expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 74 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/5
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-03 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#9).

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..

IMPALA-7031: Add additional info to query canceled from http endpoint

When a running query is canceled from the http endpoint, the query will
be unregistered. This can result in a scenario where a client that has
a reference to the query handle is unaware that the query has been
unregistered via http endpoint, and attempts to run an operation on
that query handle but encounters "Invalid query handle" error.
From the client/user prospective the error is not indicative, so the
patch added additional information to error to notify that the query is
already unregistered.

Testing:
- Added a test to test_web_pages.py
- Ran all webserver tests and all tests have been changed

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/hs2/test_hs2.py
M tests/query_test/test_cancellation.py
M tests/shell/test_shell_commandline.py
M tests/stress/concurrent_select.py
M tests/webserver/test_web_pages.py
9 files changed, 75 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/9
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 9
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-03 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#8).

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..

IMPALA-7031: Add additional info to query canceled from http endpoint

When a running query is canceled from the http endpoint, the query will
be unregistered. This can result in a scenario where a client that has
a reference to the query handle is unaware that the query has been
unregistered via http endpoint, and attempts to run an operation on
that query handle but encounters "Invalid query handle" error.
From the client/user prospective the error is not indicative, so the
patch added additional information to error to notify that the query is
already unregistered.

Testing:
- Added a test to test_web_pages.py
- Ran all webserver tests and all tests have been changed

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/hs2/test_hs2.py
M tests/query_test/test_cancellation.py
M tests/shell/test_shell_commandline.py
M tests/stress/concurrent_select.py
M tests/webserver/test_web_pages.py
9 files changed, 75 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/8
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 8
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

2019-05-02 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13050 )

Change subject: IMPALA-966: Type errors are attributed to wrong expression with 
insert
..


Patch Set 4:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/13050/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/2//COMMIT_MSG@9
PS2, Line 9:
> line wraps. Please don't exceed the col limit
Done


http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG@9
PS3, Line 9: insert multiple incompatible type values into a
> can you clarify what this mean "insert a union"?
Done


http://gerrit.cloudera.org:8080/#/c/13050/3//COMMIT_MSG@10
PS3, Line 10: error message should blame on the correct expression. If there
> multiple expressions with an incompatible type
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS3, Line 2326: expression for each column i
> return a list of the first widest compatible expression for each column
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2326
PS3, Line 2326: Retu
> where are we logging this?
updated comments. thanks!


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2340
PS3, Line 2340: Type compatibleType = firstList.get(i).getType();
> nit: dont think this context is necessary here
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2347
PS3, Line 2347: // compatibleType will be updated if a new wider type is 
encountered
> nit: spavce between // and The
Done


http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@690
PS2, Line 690:   // widestTypeExpr is widest type expression for column i
> +1. Can you simplify this?
Done


http://gerrit.cloudera.org:8080/#/c/13050/1/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/1/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@53
PS1, Line 53:
> Any reason to put this here instead of in UnionStmt?
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@53
PS3, Line 53:
:   protected List orderByElements_;
:   protected LimitElement limitElement_;
:
:   // For a select statment:
> can you rephrase this to be more concise. Please look at other comments on
Thanks. updated comments


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@58
PS3, Line 58: xprs in sele
> Like bharath mentioned, you can add this to UnionStmt instead and in Insert
Done


http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/2/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@195
PS2, Line 195:*
> Best to remove trailing spaces. Eclipse can do this for you. I think it is
Thanks Paul. it is handy!


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@196
PS3, Line 196:* widestTypeSrcExpr is the widest type expression for all the 
source values.
 :* Error message should blame on the widestTypeSrcExpr instead 
of the first
 :* compatible source
> looks like an outdated comment, can up update it based on the changes in yo
Done


http://gerrit.cloudera.org:8080/#/c/13050/3/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@232
PS3, Line 232: 
 :
 :
 :
> since checkTypeCompatibility is only used in 2 places, there is no need to
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id:

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-02 Thread Alice Fan (Code Review)

Alice Fan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12926 )

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..


Patch Set 7:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@9
PS4, Line 9: When a running query is canceled from the http endpoint, the query 
will
> nit: queries debug page
Done


http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@10
PS4, Line 10:  a client that has
: a reference to the query handle is unaware that the query has been
: unregistered via
> This can result in a scenario where a client that has a reference to the qu
Updated. Thanks. It is much clear.


http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@12
PS4, Line 12: tp endpoint, and attempts to run an operation on
: that query handle but encounters "Invalid query handle" error.
: From the clie
> This patch updates the cancel http endpoint to only cancel the query.
Done


http://gerrit.cloudera.org:8080/#/c/12926/4//COMMIT_MSG@18
PS4, Line 18: sti
> nit: ran


http://gerrit.cloudera.org:8080/#/c/12926/6/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:

http://gerrit.cloudera.org:8080/#/c/12926/6/be/src/service/impala-hs2-server.cc@179
PS6, Line 179: VLOG(1) << err_msg;
 : return Status::Expected(err_msg);
 :   }
 :
 :   //
> this looks like common code that you can refactor out and use at all other
Done


http://gerrit.cloudera.org:8080/#/c/12926/1/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/12926/1/tests/webserver/test_web_pages.py@543
PS1, Line 543:
> flake8: E501 line too long (93 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@537
PS4, Line 537: he 'krpc_address' is the krpc address of the impalad.
 : assert len(backend_row['krpc_a
> Verify that the cancel http endpoint does not unregister the query
Done


http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@540
PS4, Line 540: ddress
> cancels
Done


http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@547
PS4, Line 547:   def test_cancel_query_by_http_endpoint(self):
> assert "Query cancellation successful" in responses[0].text, responses[0].t
Thanks. Added responses[0].text as error message


http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@548
PS4, Line 548: """When a query is successfully canceled, client should be 
notified that the
> to verify that the query id is still registered
Done


http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@551
PS4, Line 551: cancels it b
> nit not necessarily the case, could be anything. Just say something like "E
Done


http://gerrit.cloudera.org:8080/#/c/12926/4/tests/webserver/test_web_pages.py@552
PS4, Line 552:   query = "select count(*) from functional.alltypes where 
bool_col = sleep(1000)"
> finally:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 03 May 2019 05:30:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-02 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#7).

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..

IMPALA-7031: Add additional info to query canceled from http endpoint

When a running query is canceled from the http endpoint, the query will
be unregistered. This can result in a scenario where a client that has
a reference to the query handle is unaware that the query has been
unregistered via http endpoint, and attempts to run an operation on
that query handle but encounters "Invalid query handle" error.
From the client/user prospective the error is not indicative, so the
patch added additional information to error to notify that the query is
already unregistered.

Testing:
- Added a test to test_web_pages.py
- Ran all webserver tests

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/webserver/test_web_pages.py
5 files changed, 57 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/7
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 7
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

2019-05-02 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#4).

Change subject: IMPALA-966: Type errors are attributed to wrong expression with 
insert
..

IMPALA-966: Type errors are attributed to wrong expression with insert

When insert multiple incompatible type values into a table,
error message should blame on the correct expression. If there
are multiple incompatible type values for a single target
column, error should blame on the first widest incompatible type
expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 73 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/4
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7031: Add additional info to query canceled from http endpoint

2019-05-01 Thread Alice Fan (Code Review)

Alice Fan has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/12926 )

Change subject: IMPALA-7031: Add additional info to query canceled from http 
endpoint
..

IMPALA-7031: Add additional info to query canceled from http endpoint

When a running query is canceled from the http endpoint, the query will
be unregistered. This can result in a scenario where a client that has
a reference to the query handle is unaware that the query has been
unregistered via http endpoint, and attempts to run an operation on
that query handle but encounters "Invalid query handle" error.
>From the client/user prospective the error is not indicative, so the
patch added additional information to error to notify that the query is
already unregistered.

Testing:
- Added a test to test_web_pages.py
- Ran all webserver tests

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/webserver/test_web_pages.py
5 files changed, 100 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/6
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 6
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

2019-04-26 Thread Alice Fan (Code Review)

Hello Bharath Vissapragada, Paul Rogers, xiaom...@cloudera.com, Bikramjeet Vig, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/13050

to look at the new patch set (#3).

Change subject: IMPALA-966: Type errors are attributed to wrong expression with 
insert
..

IMPALA-966: Type errors are attributed to wrong expression with insert

When insert a union statement with incompatible types, we should
make error message blame on correct expression. If there are multiple
incompatible types for a single column, error message should print the
widest data type expression.

Testing:
- Added tests to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 64 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/3
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 3
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-966: Type errors are attributed to wrong expression with insert

2019-04-25 Thread Alice Fan (Code Review)

Alice Fan has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13050


Change subject: IMPALA-966: Type errors are attributed to wrong expression with 
insert
..

IMPALA-966: Type errors are attributed to wrong expression with insert

When multiple incompatible values insert into a table, error should blame 
associated expression.

Testing:
- Added a test to AnalyzeStmtsTest.java

Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
6 files changed, 55 insertions(+), 17 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/13050/2
--
To view, visit http://gerrit.cloudera.org:8080/13050
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I88718fc2cbe1a492165435a542fd2d91d8693a39
Gerrit-Change-Number: 13050
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Alice Fan 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query

2019-04-09 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#5).

Change subject: IMPALA-7031: Cancel action of debug page should not unregister 
query
..

IMPALA-7031: Cancel action of debug page should not unregister query

When a running query is canceled from the Cancel button of
queries debug page (impalad WebUI), the query will be unregistered.
This can result in a scenario where a client that has a reference to the
query handle is unaware that the query has been unregistered via the
debug page, and attempts to run an operation on that query handle
but encounters and error instead.
This patch updates the cancel http endpoint to only cancel the query.

Testing:
- Added a test to test_web_pages.py
- Ran all webserver tests

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-http-handler.cc
M tests/webserver/test_web_pages.py
2 files changed, 22 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/5
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 5
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query

2019-04-04 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#3).

Change subject: IMPALA-7031: Cancel action of debug page should not unregister 
query
..

IMPALA-7031: Cancel action of debug page should not unregister query

When a running query is cancelled from the Cancel button of debug
page (impalad WebUI), it will unregister the query. But the client
is unaware of the query is unregistered, it will get error/exception
on other attempts. The patch updated the ImpalaHttpHandler to make
"Cancel" button will only cancel the query instead of
unregister it.

Testing:
- Added a test to test_web_pages.py to test if query is unregistered when 
cancel from debug page
- Run all webserver tests

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-http-handler.cc
M tests/webserver/test_web_pages.py
2 files changed, 18 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/3
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 3
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query

2019-04-04 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#4).

Change subject: IMPALA-7031: Cancel action of debug page should not unregister 
query
..

IMPALA-7031: Cancel action of debug page should not unregister query

When a running query is cancelled from the Cancel button of debug
page (impalad WebUI), it will unregister the query. But the client
is unaware of the query is unregistered, it will get error/exception
on other attempts. The patch updated the ImpalaHttpHandler to make
"Cancel" button will only cancel the query instead of
unregister it.

Testing:
- Added a test to test_web_pages.py
- Run all webserver tests

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-http-handler.cc
M tests/webserver/test_web_pages.py
2 files changed, 18 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/4
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 4
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query

2019-04-03 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12926

to look at the new patch set (#2).

Change subject: IMPALA-7031: Cancel action of debug page should not unregister 
query
..

IMPALA-7031: Cancel action of debug page should not unregister query

When a running query is cancelled from impalad WebUI (debug page),
the client will receive an invalid query handle exception on the
next fetch/getOperationStatus attempts. The patch modified
ImpalaHttpHandler, "Cancel" button will only cancel the query instead of
unregister it.

Testing:
- Added test_cancel_query_from_debug_page at test_web_pages.py

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-http-handler.cc
M tests/webserver/test_web_pages.py
2 files changed, 21 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/2
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-7031: Cancel action of debug page should not unregister query

2019-04-03 Thread Alice Fan (Code Review)

Alice Fan has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12926


Change subject: IMPALA-7031: Cancel action of debug page should not unregister 
query
..

IMPALA-7031: Cancel action of debug page should not unregister query

When a running query is cancelled from impalad WebUI (debug page),
the client will receive an invalid query handle exception on the
next fetch/getOperationStatus attempts. The patch modified
ImpalaHttpHandler, "Cancel" button will only cancel the query instead of
unregister it.

Testing:
- Added test_cancel_query_from_debug_page at test_web_pages.py

Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
---
M be/src/service/impala-http-handler.cc
M tests/webserver/test_web_pages.py
2 files changed, 21 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/12926/1
--
To view, visit http://gerrit.cloudera.org:8080/12926
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I56983d40e0542bc734ec5a66c339b5131b7b56c8
Gerrit-Change-Number: 12926
Gerrit-PatchSet: 1
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-6852: Add number of fragment instances per host to query profile

2019-03-22 Thread Alice Fan (Code Review)

Hello Bikramjeet Vig, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/12831

to look at the new patch set (#2).

Change subject: IMPALA-6852: Add number of fragment instances per host to query 
profile
..

IMPALA-6852: Add number of fragment instances per host to query profile

Query profile is updated to include number of fragment instances
per host. Example from "select count (*) from functional.alltypes":
- Per Host Number of Fragment Instances: afan-box-7060:22000(2)
  afan-box-7060:22001(1) afan-box-7060:22002(1)

Added a basic observability test to ensure that the expected section
appears in the query profile.

Change-Id: I9556d7f1d56fd5c68cc2fb60df6f745a74cd4879
---
M be/src/scheduling/scheduler.cc
M tests/query_test/test_observability.py
2 files changed, 14 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/12831/2
--
To view, visit http://gerrit.cloudera.org:8080/12831
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9556d7f1d56fd5c68cc2fb60df6f745a74cd4879
Gerrit-Change-Number: 12831
Gerrit-PatchSet: 2
Gerrit-Owner: Alice Fan 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins

55 matches

Mail list logo