Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8172 )
Change subject: IMPALA-5307: Part 4: copy out uncompressed text and seq ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-scan-node.cc@238 PS8, Line 238: > nit: a lot of unnecessary blank lines. maybe condense a bit so more code fi Done http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc File be/src/exec/hdfs-sequence-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-sequence-scanner.cc@329 PS8, Line 329: add_row = WriteCompleteTuple(row_batch->tuple_data_pool(), field_locations_.data(), > I missed updating this additional code path, we also need to copy out strin Done. The below test was previously failing but now succeeds. We should get better coverage of some of these things once we switch the I/O mgr to the buffer pool and have ASAN poisoning enabled for recycled buffers. ./buildall.sh -asan -skiptests -noclean -ninja -notests && start-impala-cluster.py --impalad_args=--disable_mem_pools=true && impala-py.test -n1 --verbose tests/query_test/test_scanners.py tests/query_test/test_aggregation.py --workload_exploration_strategy=functional-query:exhaustive -k seq http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8172/8/be/src/exec/hdfs-text-scanner.cc@704 PS8, Line 704: bool split_delimiter_possible = context_->partition_descriptor()->line_delim() == '\n' > I think there's a latent bug here where we're touching byte_buffer_ *after* I filed IMPALA-6137. I think there are pre-existing bugs here so won't tackle them in this patch (this patch may have to wait for those though since it increase the odds of hitting them). -- To view, visit http://gerrit.cloudera.org:8080/8172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I304fd002b61bfedf41c8b1405cd7eb7b492bb941 Gerrit-Change-Number: 8172 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 31 Oct 2017 21:48:42 +0000 Gerrit-HasComments: Yes
