impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-(3895,3859): Don't log file data on parse errors
Date Thu, 18 Aug 2016 20:30:49 GMT
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 <henry@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message