Dan Hecht has posted comments on this change.

Change subject: IMPALA-3764,3914: fuzz test HDFS scanners and fix parquet bugs 
found
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3448/9/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

Line 649:       FILE_CHECK_GE(col_reader->def_level(),
> I'm not sure I exactly understand. FILE_CHECKs are DCHECKs now so we should
They are implemented as DCHECKs, but they aren't invariants since they depend 
on the values in the input file.  So, if they are conditions that downstream 
code depends on, they should really be runtime checks.  There existence implies 
a path that is possible with a corrupt file that we don't exercise.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I50cf43195a7c582caa02c85ae400ea2256fa3a3b
Gerrit-PatchSet: 9
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Taras Bobrovytsky <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to