[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8719/6/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/8719/6/tests/metadata/test_ddl.py@353
PS6, Line 353: assert '\n'.join(plan.data[4:]) == \
Why does python IN not work? This will break if we change the explain header.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 6
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 11 Dec 2017 18:27:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 7: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8719/7/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/8719/7/tests/metadata/test_ddl.py@362
PS7, Line 362: 00:SCAN HDFS [functional.alltypestiny a]""" in 
'\n'.join(plan.data)
> I think you meant this?
Works for me. My primary concern is with sprinkling expected explain plans in 
various tests. That makes it really hard to change anything in the planner 
because "random" unrelated tests can fail. So when checking plans we should 
minimize the ways in which planner changes can break it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 7
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 11 Dec 2017 19:18:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 7
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Tue, 12 Dec 2017 00:17:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..

IMPALA-6245: Tolerate column indenting from Hive

The fix for HIVE-3140 started indenting multi-line comments,
which breaks Impala testing when run against Hive 2.1.1.

To test this using the pure test runner proved difficult
since it would require extensive changes to support both
row_regexes (since the columns changed order) and subset
support (since the number of rows changed).

Instead, we manually verify the hints are present in the
output in the python test.

The fact that the hints have been reformatted leaves us
in an uncertain state as to whether they actually get applied,
so a new test case has been added to run EXPLAIN SELECT
on the view and verify the joins happen exactly as we
expect.

Testing: Ran the views-ddl test against Impala mini-cluster
setups using both Hive 2.1.1 and Hive 1.1.0

Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Reviewed-on: http://gerrit.cloudera.org:8080/8719
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
M tests/metadata/test_ddl.py
2 files changed, 42 insertions(+), 39 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 8
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 7
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 11 Dec 2017 20:40:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Zach Amsden (Code Review)]

Hello Philip Zeyliger, David Knupp, Joe McDonnell, Alex Behm,

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

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

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

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..

IMPALA-6245: Tolerate column indenting from Hive

The fix for HIVE-3140 started indenting multi-line comments,
which breaks Impala testing when run against Hive 2.1.1.

To test this using the pure test runner proved difficult
since it would require extensive changes to support both
row_regexes (since the columns changed order) and subset
support (since the number of rows changed).

Instead, we manually verify the hints are present in the
output in the python test.

The fact that the hints have been reformatted leaves us
in an uncertain state as to whether they actually get applied,
so a new test case has been added to run EXPLAIN SELECT
on the view and verify the joins happen exactly as we
expect.

Testing: Ran the views-ddl test against Impala mini-cluster
setups using both Hive 2.1.1 and Hive 1.1.0

Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
---
M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
M tests/metadata/test_ddl.py
2 files changed, 42 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 7
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Zach Amsden (Code Review)]

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8719/6/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/8719/6/tests/metadata/test_ddl.py@353
PS6, Line 353: assert '\n'.join(plan.data[4:]) == \
> Why does python IN not work? This will break if we change the explain heade
IN works fine on the flattened plan.  For some reason I thought you were asking 
for IN on the list.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 6
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 11 Dec 2017 18:59:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Zach Amsden (Code Review)]

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8719/5/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/8719/5/tests/metadata/test_ddl.py@353
PS5, Line 353: assert '\n'.join(plan.data) == \
> Let's do contains and not include the first 4 expected lines since those mi
Contains is harder than expected in Python and also a weaker condition.  Slices 
to the rescue!: '\n'.join(plan.data[4:])



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 5
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 11 Dec 2017 18:12:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Zach Amsden (Code Review)]

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
File testdata/workloads/functional-query/queries/QueryTest/views-ddl.test:

http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@281
PS4, Line 281:  QUERY
> Move this and the query validation into Python as well. We don't want to ha
Done


http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@289
PS4, Line 289: # Test that the plan respects the plan hints for shuffle and 
broadcast
> I suggest moving this into Python as well. Might want to use explain_level=
Done


http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@327
PS4, Line 327: # Test querying the hinted view.
> Move into Python as well.
Done


http://gerrit.cloudera.org:8080/#/c/8719/1/tests/common/test_result_verifier.py
File tests/common/test_result_verifier.py:

http://gerrit.cloudera.org:8080/#/c/8719/1/tests/common/test_result_verifier.py@100
PS1, Line 100:   'Number of columns returned > the number of column 
types: %s' % column_types
Garbage, will post another diff



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 4
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 08 Dec 2017 23:47:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
File testdata/workloads/functional-query/queries/QueryTest/views-ddl.test:

http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@281
PS4, Line 281:  QUERY
Move this and the query validation into Python as well. We don't want to have a 
dependency on a .test file producing something that is then later used in 
Python. Better to have the whole test including setup and validation 
self-contained in one place.


http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@289
PS4, Line 289: # Test that the plan respects the plan hints for shuffle and 
broadcast
I suggest moving this into Python as well. Might want to use explain_level=0 to 
keep it short.


http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@327
PS4, Line 327: # Test querying the hinted view.
Move into Python as well.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 4
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 06 Dec 2017 05:14:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Zach Amsden (Code Review)]

Zach Amsden has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..

IMPALA-6245: Tolerate column indenting from Hive

The fix for HIVE-3140 started indenting multi-line comments,
which breaks Impala testing when run against Hive 2.1.1.

To test this using the pure test runner proved difficult
since it would require extensive changes to support both
row_regexes (since the columns changed order) and subset
support (since the number of rows changed).

Instead, we manually verify the hints are present in the
output in the python test.

The fact that the hints have been reformatted leaves us
in an uncertain state as to whether they actually get applied,
so a new test case has been added to run EXPLAIN SELECT
on the view and verify the joins happen exactly as we
expect.

Testing: Ran the views-ddl test against Impala mini-cluster
setups using both Hive 2.1.1 and Hive 1.1.0

Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
---
M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
M tests/metadata/test_ddl.py
2 files changed, 48 insertions(+), 20 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 3
Gerrit-Owner: Zach Amsden 

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Zach Amsden (Code Review)]

Zach Amsden has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..

IMPALA-6245: Tolerate column indenting from Hive

The fix for HIVE-3140 started indenting multi-line comments,
which breaks Impala testing when run against Hive 2.1.1.

To test this using the pure test runner proved difficult
since it would require extensive changes to support both
row_regexes (since the columns changed order) and subset
support (since the number of rows changed).

Instead, we manually verify the hints are present in the
output in the python test.

The fact that the hints have been reformatted leaves us
in an uncertain state as to whether they actually get applied,
so a new test case has been added to run EXPLAIN SELECT
on the view and verify the joins happen exactly as we
expect.

Testing: Ran the views-ddl test against Impala mini-cluster
setups using both Hive 2.1.1 and Hive 1.1.0

Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
---
M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
M tests/metadata/test_ddl.py
2 files changed, 50 insertions(+), 22 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 2
Gerrit-Owner: Zach Amsden 

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

[Zach Amsden (Code Review)]

Zach Amsden has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8719


Change subject: IMPALA-6245: Tolerate column indenting from Hive
..

IMPALA-6245: Tolerate column indenting from Hive

The fix for HIVE-3140 started indenting multi-line comments,
which breaks Impala testing when run against Hive 2.1.1.

To test this using the pure test runner proved difficult
since it would require extensive changes to support both
row_regexes (since the columns changed order) and subset
support (since the number of rows changed).

Instead, we manually verify the hints are present in the
output in the python test.

The fact that the hints have been reformatted leaves us
in an uncertain state as to whether they actually get applied,
so a new test case has been added to run EXPLAIN SELECT
on the view and verify the joins happen exactly as we
expect.

Testing: Ran the views-ddl test against Impala mini-cluster
setups using both Hive 2.1.1 and Hive 1.1.0

Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
---
M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
M tests/common/test_result_verifier.py
M tests/metadata/test_ddl.py
3 files changed, 51 insertions(+), 23 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 1
Gerrit-Owner: Zach Amsden