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

[Impala Public Jenkins (Code Review)]

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 
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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 

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

[Impala Public Jenkins (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Oct 2017 20:32:27 +
Gerrit-HasComments: No

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

[Tim Armstrong (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Oct 2017 20:32:19 +
Gerrit-HasComments: No

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Oct 2017 19:39:13 +
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 05 Oct 2017 01:32:25 +
Gerrit-HasComments: Yes

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

[Tim Armstrong (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 04 Oct 2017 21:37:22 +
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 03 Oct 2017 23:45:55 +
Gerrit-HasComments: Yes

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

[Tim Armstrong (Code Review)]

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 (:<# splits>/): -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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 03 Oct 2017 18:38:43 +
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 03 Oct 2017 00:57:05 +
Gerrit-HasComments: Yes

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

[Joe McDonnell (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 02 Oct 2017 22:05:50 +
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Tue, 26 Sep 2017 17:29:39 +
Gerrit-HasComments: Yes

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

[Joe McDonnell (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Mon, 25 Sep 2017 23:25:30 +
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Joe McDonnell (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

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

[Joe McDonnell (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

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

[Joe McDonnell (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh

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

[Pranay Singh (Code Review)]

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 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

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

[Joe McDonnell (Code Review)]

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  

Where  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 
Gerrit-HasComments: Yes

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

[Pranay Singh (Code Review)]

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