You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Alex Behm (Code Review)" <ge...@cloudera.org> on 2018/02/26 20:06:24 UTC

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

Alex Behm has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9453


Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................

IMPALA-6586: Fix bug in TestGetTablesTypeTable()

The bug in FrontendTest.TestGetTablesTypeTable() was
that it did not explicitly load views that the test
assumed to be loaded already. The test needs to
distinguish between views and tables and views need
to be loaded for them to be discernable from tables.

I was able to reproduce the issue localy by just
running FrontendTest.TestGetTablesTypeTable() without
any other test.

Testing:
- locally ran all tests in FrontendTest individually
  (with a fresh ImpaladTestCatalog)

Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
---
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
1 file changed, 7 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm <al...@cloudera.com>

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

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

Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9453/1/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/9453/1/fe/src/test/java/org/apache/impala/service/FrontendTest.java@111
PS1, Line 111:     AnalyzesOk("select * from functional.alltypes_view_sub");
> I think this could fail again if someone adds a new view to functional sche
I was thinking about this too and concluded that we'd end up having to change this test unless we make the test so generic that it's hard to understand.


http://gerrit.cloudera.org:8080/#/c/9453/1/fe/src/test/java/org/apache/impala/service/FrontendTest.java@124
PS1, Line 124:     assertEquals("alltypes_datasource", resp.rows.get(0).colVals.get(2).string_val.toLowerCase());
> Not related to your patch, but maybe we can fix the overflow here and below
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2018 04:52:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, 

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

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

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

Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................

IMPALA-6586: Fix bug in TestGetTablesTypeTable()

The bug in FrontendTest.TestGetTablesTypeTable() was
that it did not explicitly load views that the test
assumed to be loaded already. The test needs to
distinguish between views and tables and views need
to be loaded for them to be discernable from tables.

I was able to reproduce the issue localy by just
running FrontendTest.TestGetTablesTypeTable() without
any other test.

Testing:
- locally ran all tests in FrontendTest individually
  (with a fresh ImpaladTestCatalog)

Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
---
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
1 file changed, 17 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

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

Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2020/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 27 Feb 2018 04:54:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

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

Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2018 04:53:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

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

Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 27 Feb 2018 08:34:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

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

Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................


Patch Set 1: Code-Review+2

(2 comments)

Seems simple enough to +2.

http://gerrit.cloudera.org:8080/#/c/9453/1/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/9453/1/fe/src/test/java/org/apache/impala/service/FrontendTest.java@111
PS1, Line 111:     AnalyzesOk("select * from functional.alltypes_view_sub");
I think this could fail again if someone adds a new view to functional schema, but I can't think of a better solution :)


http://gerrit.cloudera.org:8080/#/c/9453/1/fe/src/test/java/org/apache/impala/service/FrontendTest.java@124
PS1, Line 124:     assertEquals("alltypes_datasource", resp.rows.get(0).colVals.get(2).string_val.toLowerCase());
Not related to your patch, but maybe we can fix the overflow here and below while we are here? Thanks



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2018 00:17:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6586: Fix bug in TestGetTablesTypeTable()

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9453 )

Change subject: IMPALA-6586: Fix bug in TestGetTablesTypeTable()
......................................................................

IMPALA-6586: Fix bug in TestGetTablesTypeTable()

The bug in FrontendTest.TestGetTablesTypeTable() was
that it did not explicitly load views that the test
assumed to be loaded already. The test needs to
distinguish between views and tables and views need
to be loaded for them to be discernable from tables.

I was able to reproduce the issue localy by just
running FrontendTest.TestGetTablesTypeTable() without
any other test.

Testing:
- locally ran all tests in FrontendTest individually
  (with a fresh ImpaladTestCatalog)

Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Reviewed-on: http://gerrit.cloudera.org:8080/9453
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
1 file changed, 17 insertions(+), 6 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idf0bddb2e29209adda5bda5ddc428f46f241c8c9
Gerrit-Change-Number: 9453
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins