Dan Hecht has posted comments on this change. Change subject: IMPALA-4371: Incorrect DCHECK-s in hdfs-parquet-table-writer ......................................................................
Patch Set 2: Code-Review+2 > > Is it reasonably possible to create a file that demonstrates this > > and can be added to the regression test suite? > > Dan, it has just occurred to me that since this condition happens > while writing a file and not while reading it, I don't see how we > could use a test file to test it. Additionally, I don't see a real > risk of regression, as I don't expect anyone to put in DCHECK-s > based on the same false assumptions again. > > So considering the value of such a test and the difficulty of > reproducing the exact conditions triggering the DCHECK-s, I suggest > not creating a test file for the regression suite. Couldn't you just hack up the Impala parquet writer (or any other parquet writing util) to generate such a parquet file to use as a test? That said, given the trivial nature of the change and the zero risk, I'm okay with deferring that. Still would be nice to exercise this path, though, in case there is another problem downstream from this code that such a file would encounter. Do you have access to the file to repro'ed this in the wild? And given that DCHECKs are compiled out of release builds, how did this cause a problem in the wild? -- To view, visit http://gerrit.cloudera.org:8080/4835 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I52e1b1354e9ea056b49331e75e53759952a81b76 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zoltan Ivanfi <z...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Ivanfi <z...@cloudera.com> Gerrit-HasComments: No
