You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Steve Carlin (Code Review)" <ge...@cloudera.org> on 2021/06/14 16:30:52 UTC

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility 

Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17590


Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................

IMPALA-10750: Impala-shell changes for HS2 compatibility

Need some changes to impala-shell to make the client more HS2 compatible,
including:

- when the fetch returns the bitset containing nulls, the lack of presence of
  bits means it is not null.  Currently it will fail the query.

- adding fetchType to TCLIServiceThrift structure (though unused currently
  in Impala)

Also a small refactor was done to put the functionality that retrieves
all query options into its own function.

Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
---
M common/thrift/hive-1-api/TCLIService.thrift
M shell/impala_client.py
2 files changed, 14 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Jun 2021 17:45:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................

IMPALA-10750: Impala-shell changes for HS2 compatibility

Need some changes to impala-shell to make the client more HS2
compatible, including:

- when the fetch returns the bitset containing nulls, the lack of
  presence of bits means it is not null.  Currently it will fail
  the query.

- adding fetchType to TCLIServiceThrift structure (though unused
  currently in Impala)

Also a small refactor was done to put the functionality that retrieves
all query options into its own function.

Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Reviewed-on: http://gerrit.cloudera.org:8080/17590
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Csaba Ringhofer <cs...@cloudera.com>
---
M common/thrift/hive-1-api/TCLIService.thrift
M shell/impala_client.py
2 files changed, 38 insertions(+), 23 deletions(-)

Approvals:
  Csaba Ringhofer: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 12
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8940/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 16 Jun 2021 16:32:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Attila Jeges, Csaba Ringhofer, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................

IMPALA-10750: Impala-shell changes for HS2 compatibility

Need some changes to impala-shell to make the client more HS2
compatible, including:

- when the fetch returns the bitset containing nulls, the lack of
  presence of bits means it is not null.  Currently it will fail
  the query.

- adding fetchType to TCLIServiceThrift structure (though unused
  currently in Impala)

Also a small refactor was done to put the functionality that retrieves
all query options into its own function.

Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
---
M common/thrift/hive-1-api/TCLIService.thrift
M shell/impala_client.py
2 files changed, 38 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17590/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17590/3/shell/impala_client.py@855
PS3, Line 855: r
flake8: E501 line too long (99 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 16 Jun 2021 16:11:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................

IMPALA-10750: Impala-shell changes for HS2 compatibility

Need some changes to impala-shell to make the client more HS2 compatible,
including:

- when the fetch returns the bitset containing nulls, the lack of presence of
  bits means it is not null.  Currently it will fail the query.

- adding fetchType to TCLIServiceThrift structure (though unused currently
  in Impala)

Also a small refactor was done to put the functionality that retrieves
all query options into its own function.

Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
---
M common/thrift/hive-1-api/TCLIService.thrift
M shell/impala_client.py
2 files changed, 29 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................

IMPALA-10750: Impala-shell changes for HS2 compatibility

Need some changes to impala-shell to make the client more HS2 compatible,
including:

- when the fetch returns the bitset containing nulls, the lack of presence of
  bits means it is not null.  Currently it will fail the query.

- adding fetchType to TCLIServiceThrift structure (though unused currently
  in Impala)

Also a small refactor was done to put the functionality that retrieves
all query options into its own function.

Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
---
M common/thrift/hive-1-api/TCLIService.thrift
M shell/impala_client.py
2 files changed, 15 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py@846
PS2, Line 846: is_null.frombytes(tcol.nulls)
> Would it be possible for tcol.nulls to be None (no NULL values)? If so, thi
I'm not seeing the whole value as None.  In the case where there are no Null values, I see an empty bit set.  So I think this is ok.


http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py@851
PS2, Line 851: enumerate
> I am a bit concerned about the performance impact of adding an extra test. 
My python knowledge isn't the greatest...if you feel this is too verbose, let's discuss.


http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py@856
PS2, Line 856:           row[col_idx] = 'NULL' if is_null[row_idx] else stringifier(tcol.values[row_idx])
> the fix above is needed here too
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 15 Jun 2021 16:42:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Jun 2021 17:45:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................

IMPALA-10750: Impala-shell changes for HS2 compatibility

Need some changes to impala-shell to make the client more HS2 compatible,
including:

- when the fetch returns the bitset containing nulls, the lack of presence of
  bits means it is not null.  Currently it will fail the query.

- adding fetchType to TCLIServiceThrift structure (though unused currently
  in Impala)

Also a small refactor was done to put the functionality that retrieves
all query options into its own function.

Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
---
M common/thrift/hive-1-api/TCLIService.thrift
M shell/impala_client.py
2 files changed, 24 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17590/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17590/1/shell/impala_client.py@852
PS1, Line 852: a
flake8: E501 line too long (104 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Jun 2021 16:31:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has removed a vote on this change.

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
-- 
To view, visit http://gerrit.cloudera.org:8080/17590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8908/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Jun 2021 19:40:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py@846
PS2, Line 846: is_null.frombytes(tcol.nulls)
Would it be possible for tcol.nulls to be None (no NULL values)? If so, this line will raise an exception.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 15 Jun 2021 13:26:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8905/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 14 Jun 2021 16:53:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17590/4/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17590/4/shell/impala_client.py@853
PS4, Line 853: r
flake8: E501 line too long (99 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 16 Jun 2021 16:11:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8984/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 11
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jun 2021 18:17:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 2:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/17590/2//COMMIT_MSG@9
PS2, Line 9: Need some changes to impala-shell to make the client more HS2 compatible,
nit: please wrap at 72 chars


http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py@851
PS2, Line 851: enumerate
I am a bit concerned about the performance impact of adding an extra test. It may be faster if we enumerate only on the first len(is_null) of rows and go through the rest of the values in another loop.


http://gerrit.cloudera.org:8080/#/c/17590/2/shell/impala_client.py@856
PS2, Line 856:           row[col_idx] = 'NULL' if is_null[row_idx] else stringifier(tcol.values[row_idx])
the fix above is needed here too



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 15 Jun 2021 13:28:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10750: Impala-shell changes for HS2 compatibility

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17590 )

Change subject: IMPALA-10750: Impala-shell changes for HS2 compatibility
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8941/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3a4c4ce8a5d60db136df1743f32dba22172ee13
Gerrit-Change-Number: 17590
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Wed, 16 Jun 2021 16:32:37 +0000
Gerrit-HasComments: No