Tim Armstrong has posted comments on this change. Change subject: IMPALA-(3895,3859): Don't log file data on parse errors ......................................................................
Patch Set 2: (4 comments) It's kind of unfortunate to lose the diagnostic info if people are running without security. It seems like it would be significantly less convenient to diagnose if ingested text files weren't being parsed as expected by Impala. It doesn't seem like there's a simple way to determine if it's safe to expose table information unless I'm missing something, so this is probably the best way forward. http://gerrit.cloudera.org:8080/#/c/4020/2/be/src/exec/hdfs-scanner.cc File be/src/exec/hdfs-scanner.cc: PS2, Line 624: const string& s A string reference seems wrong here - where is the string it's referencing actually stored? PS2, Line 635: TO weird caps - should be lower case? http://gerrit.cloudera.org:8080/#/c/4020/2/testdata/workloads/functional-query/queries/DataErrorsTest/avro-errors.test File testdata/workloads/functional-query/queries/DataErrorsTest/avro-errors.test: PS2, Line 11: NAMENODE I find NAMENODE a little confusing since there is no namenode for many filesystems. Should it be something like FS_URL or FILESYSTEM_URL or FS_BASE_URL? http://gerrit.cloudera.org:8080/#/c/4020/2/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: Line 328: replace_filenames_with_placeholder = True I think like this is getting big enough to be its own function. E.g. verify_results_section() -- To view, visit http://gerrit.cloudera.org:8080/4020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5a604f8784a9ff7b4bf878f82ee7f56697df3272 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
