You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pranay Singh (Code Review)" <ge...@cloudera.org> on 2017/09/13 18:11:55 UTC

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Pranay Singh has uploaded a new change for review.

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 13 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 9: Code-Review+2

Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 9
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Oct 2017 20:32:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8056/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8056/8//COMMIT_MSG@15
PS8, Line 15: Ran the query_test/test_scanners_fuzz.py in a loop (5 times)
> I was thinking more like overnight (or a few hours). It's worth doing just 
OK I'll  run overnight and check tomorrow morning what happens.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 8
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Oct 2017 01:32:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has posted comments on this change.

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8056/1/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

Line 68
> Avoid unrelated whitespace diffs.
Thanks I was able to fix this issue


PS1, Line 96:     fq_tbl_name = "functional_parquet" + "." + tbl_name
> I'm wary of creating tables in our default schemas. This won't get cleaned 
Shall I drop the table after running fuzz test ?


PS1, Line 98:     create = ("create table {0} stored as parquet as select * from functional.alltypes"
            :               .format(fq_tbl_name))
> I think we need to verify that the right options are being set when we crea
I introduced a check that compression_codec == none is only used here  line #93


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#2).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 14 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#4).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 59 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#3).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 59 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#2).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 14 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8056/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8056/6//COMMIT_MSG@12
PS6, Line 12: 
> Can you mention what testing you did? The fuzz tests are randomised we shou
Done


http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py@101
PS6, Line 101:     """Parquet tables in default schema are compressed, so in order
> It's weird that parquet/none means parquet/snappy. Unsure what the history 
Added a comment stating that parquet/none means default SNAPPY compression is set.


http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py@116
PS6, Line 116:                     " select * from functional_parquet.{1}".format(fq_tbl_name, orig_tbl_name))
> Long lines > 90 chars here and just below.
Fixed it.


http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py@140
PS6, Line 140:     self.execute_query("create table %s.%s like %s.%s" % (fuzz_db, fuzz_table, src_db, src_table))
> Long line
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 6
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Oct 2017 23:45:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8056/2/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

PS2, Line 97: """
> I'll use the functional-parquet to create the cloned table which will be pa
One thing that I failed to mention clearly enough is that the uncompressed parquet table should not be created in the default schemas. Instead, it should be created in unique_database. This is why the function signature of run_fuzz_test will change. The normal tests will pass in table_format and a table from the default schema. Your new test will pass in unique_database and the table you created.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#3).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 23 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#3).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 24 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8056/2/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

PS2, Line 97: fq_tbl_name = "functional_parquet" + "." + tbl_name
> Do not create a table in the default schemas. Instead, with the changes I d
Another option is to keep the clone code in run_fuzz_test, but change the arguments so that it specifies a source database, source table, destination database, and destination table. The existing code would simply pass in the appropriate existing table. The uncompressed parquet code would create an uncompressed parquet table and pass that in.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#5).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 59 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Tim Armstrong, 

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

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

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Testing
-------
Ran the query_test/test_scanners_fuzz.py in a loop (5 times) and there was no impalad
crash seen.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 50 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 7
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8056/5/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8056/5/tests/query_test/test_scanners_fuzz.py@103
PS5, Line 103: with no compression.
> Nit: I would emphasize that this has a new table. Something like: "into a n
Done


http://gerrit.cloudera.org:8080/#/c/8056/5/tests/query_test/test_scanners_fuzz.py@110
PS5, Line 110: for orig_tbl_name in tbl_list:
             :       src_table_name = "parquet_uncomp_src_" + orig_tbl_name
             :       dst_table_name = "parquet_uncomp_dst_" + orig_tbl_name
> Nit:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 5
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Oct 2017 00:57:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 9
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Oct 2017 00:32:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 2:

(2 comments)

Take a look at these suggestions and let me know if they make sense.

http://gerrit.cloudera.org:8080/#/c/8056/2/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

PS2, Line 97: fq_tbl_name = "functional_parquet" + "." + tbl_name
Do not create a table in the default schemas. Instead, with the changes I described elsewhere, create a separate clone function that will create an uncompressed parquet table directly in unique_database and then pass that into run_fuzz_test.

The existing tests will run a simple table clone function that is equivalent to the current code.


PS2, Line 129:     self.execute_query("create table %s.%s like %s" % (unique_database, table, table))
             :     fuzz_table_location = get_fs_path("/test-warehouse/{0}.db/{1}".format(
             :         unique_database, table))
Pull this logic out into its own function e.g. clone_table. run_fuzz_test should take in a table that has already been created in unique_database. It should not do the clone itself. This allows you to use a different clone function to create a parquet table without compression.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, 

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

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

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 42 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 5
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, 

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

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

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 46 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8056/1/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

Line 68
Avoid unrelated whitespace diffs.

One way of getting a graphical diff that can help with this is to use the tool meld. For example:

git difftool -y -t meld <branch> <file>

Where <branch> could be asf-gerrit/master or origin/master or whatnot.


PS1, Line 96:     fq_tbl_name = "functional_parquet" + "." + tbl_name
I'm wary of creating tables in our default schemas. This won't get cleaned up, and it is subtle behavior. If we can create the new table in the unique_database that would be nice


PS1, Line 98:     create = ("create table {0} stored as parquet as select * from functional.alltypes"
            :               .format(fq_tbl_name))
I think we need to verify that the right options are being set when we create this table. As I understand it, you need to specify the query option compression_codec = none to create a parquet file without compression. 

https://www.cloudera.com/documentation/enterprise/5-8-x/topics/impala_compression_codec.html


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 5:

(2 comments)

I'm close to a +1 on this. Only minor thoughts left for me. I just invited Tim to the review.

http://gerrit.cloudera.org:8080/#/c/8056/5/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8056/5/tests/query_test/test_scanners_fuzz.py@103
PS5, Line 103: with no compression.
Nit: I would emphasize that this has a new table. Something like: "into a new table with no compression"


http://gerrit.cloudera.org:8080/#/c/8056/5/tests/query_test/test_scanners_fuzz.py@110
PS5, Line 110: for orig_tbl_name in tbl_list:
             :       src_table_name = "parquet_uncomp_src_" + orig_tbl_name
             :       dst_table_name = "parquet_uncomp_dst_" + orig_tbl_name
Nit:
I think I would prefer to have "dst_table_name" be "fuzz_table_name". Also, in other code, I think it would be good to be explicit about this being the fuzz_db and fuzz_table rather than dst_db and dst_table.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 5
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Oct 2017 22:05:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 6:

(4 comments)

The test doesn't appear to be creating uncompressed parquet files. Looking at the query profile scanning test_fuzz_uncompressed_parquet_fc4a3734.parquet_uncomp_dst_decimal_tbl I see:


        HDFS_SCAN_NODE (id=0):(Total: 5.970ms, non-child: 5.970ms, % non-child: 100.00%)
          Hdfs split stats (<volume id>:<# splits>/<split lengths>): -1:1/1.43 KB 
          ExecOption: PARQUET Codegen Enabled, Codegen enabled: 1 out of 1
          Hdfs Read Thread Concurrency Bucket: 0:0% 1:0% 2:0% 3:0% 4:0% 5:0% 6:0% 7:0% 
          File Formats: PARQUET/SNAPPY:6 

It looks like compression_codec isn't modified so we're just getting the default behaviour of using snappy compression.

http://gerrit.cloudera.org:8080/#/c/8056/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8056/6//COMMIT_MSG@12
PS6, Line 12: 
Can you mention what testing you did? The fuzz tests are randomised we should run them in a loop for a while to confirm that that they're stable.


http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py@101
PS6, Line 101:     """Parquet tables in default schema are compressed, so in order
It's weird that parquet/none means parquet/snappy. Unsure what the history is here and we don't need to change it but it is confusing.


http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py@116
PS6, Line 116:                     " select * from functional_parquet.{1}".format(fq_tbl_name, orig_tbl_name))
Long lines > 90 chars here and just below.


http://gerrit.cloudera.org:8080/#/c/8056/6/tests/query_test/test_scanners_fuzz.py@140
PS6, Line 140:     self.execute_query("create table %s.%s like %s.%s" % (fuzz_db, fuzz_table, src_db, src_table))
Long line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 6
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Oct 2017 18:38:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 8: Code-Review+2

(1 comment)

Looks good once you've done a little more testing.

http://gerrit.cloudera.org:8080/#/c/8056/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8056/8//COMMIT_MSG@15
PS8, Line 15: Ran the query_test/test_scanners_fuzz.py in a loop (5 times)
I was thinking more like overnight (or a few hours). It's worth doing just to be sure that we won't break the build for anyone else.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 8
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Oct 2017 21:37:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 4:

(4 comments)

I like the overall approach. I have a few small naming/style issues, but I think this is getting closer.

http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@101
PS4, Line 101:     """ Clone an existing parquet table with codec as none in the
             :         unique database. This cloned table is passed to run_fuzz_test
             :         which clones the table and corrupts the table. The test later
             :         checks that there is no crash while performing SQL queries on
             :         a corrupt table.
             :     """
I think this comment should focus on why this test is different from the others. For example, it should explain that the parquet tables in the default schema are always compressed. So, in order to test uncompressed parquet, we need to create a new source table. I think you can skip the last two sentences.


http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@111
PS4, Line 111:     db_name = unique_database
I would prefer to emphasize that the source and destination are the unique_database. To make that clearer, I think I would get rid of this variable and just use unique_database directly in each location.


http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@117
PS4, Line 117: functional_parquet.alltypes
Can we extend this to do fuzzing on decimal_tbl as well? I was thinking this could be a loop that runs fuzzing over a list of tables (that happens to have two entries).


http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@118
PS4, Line 118: .format(fq_tbl_name))
This indentation is a bit awkward. I don't think .format should be on its own line. One way to get around this is to use only 4 space indentation for the second line (" select"...) and then put the .format on that line.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Mon, 25 Sep 2017 23:25:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 9
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Oct 2017 20:32:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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/8056 )

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Testing
-------
Ran the query_test/test_scanners_fuzz.py in a loop (5 times)
and there was no impalad crash seen.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Reviewed-on: http://gerrit.cloudera.org:8080/8056
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 50 insertions(+), 16 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 10
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8056/2/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8056/2/tests/query_test/test_scanners_fuzz.py@97
PS2, Line 97: if table_format.file_format != 'parquet': pytest.sk
> Another option is to keep the clone code in run_fuzz_test, but change the a
Done


http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@101
PS4, Line 101:     """ Clone an existing parquet table with codec as none in the
             :         unique database. This cloned table is passed to run_fuzz_test
             :         which clones the table and corrupts the table. The test later
             :         checks that there is no crash while performing SQL queries on
             :         a corrupt table.
             :     """
> I think this comment should focus on why this test is different from the ot
Done


http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@111
PS4, Line 111:     db_name = unique_database
> I would prefer to emphasize that the source and destination are the unique_
Done


http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@117
PS4, Line 117: functional_parquet.alltypes
> Can we extend this to do fuzzing on decimal_tbl as well? I was thinking thi
Done


http://gerrit.cloudera.org:8080/#/c/8056/4/tests/query_test/test_scanners_fuzz.py@118
PS4, Line 118: .format(fq_tbl_name))
> This indentation is a bit awkward. I don't think .format should be on its o
Moved the format in the same line as select.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Tue, 26 Sep 2017 17:29:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8056/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8056/8//COMMIT_MSG@15
PS8, Line 15: Ran the query_test/test_scanners_fuzz.py in a loop (5 times)
> OK I'll  run overnight and check tomorrow morning what happens.
Ran for 4 hours in a loop didn't see impalad crashing.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 8
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Oct 2017 19:39:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Tim Armstrong, 

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

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

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 43 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 6
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has uploaded a new patch set (#3).

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 24 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Hello Joe McDonnell, Tim Armstrong, 

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

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

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

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................

IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

test_scanners_fuzz.py currently tests compressed parquet but
does not test uncompressed parquet. This fix adds a new test
case for uncompressed parquet.

Testing
-------
Ran the query_test/test_scanners_fuzz.py in a loop (5 times)
and there was no impalad crash seen.

Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 50 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-Change-Number: 8056
Gerrit-PatchSet: 8
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet

Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
Pranay Singh has posted comments on this change.

Change subject: IMPALA-5525 Extend TestScannersFuzzing to test uncompressed parquet
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8056/2/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

PS2, Line 97: fq_tbl_name = "functional_parquet" + "." + tbl_name
> Do not create a table in the default schemas. Instead, with the changes I d
I'll use the functional-parquet to create the cloned table which will be passed to fuzz_test


PS2, Line 129:     self.execute_query("create table %s.%s like %s" % (unique_database, table, table))
             :     fuzz_table_location = get_fs_path("/test-warehouse/{0}.db/{1}".format(
             :         unique_database, table))
> Pull this logic out into its own function e.g. clone_table. run_fuzz_test s
I'll retain the old functionality as per your latest comments


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I760de7203a51cf82b16016fa8043cadc7c8325bc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes