Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16059 )

Change subject: IMPALA-9747: More fine-grained codegen for text file scanners
......................................................................


Patch Set 8: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/hdfs-scanner.cc@490
PS8, Line 490:         // TODO: Should we log this?
             :         const SlotDescriptor* slot_desc = 
node->materialized_slots_[slot_idx];
             :         VLOG(google::INFO)
             :             << "Going to call interpreted code from codegen for 
column type "
             :             << slot_desc->type() << ".";
I don't think that we need to log this.


http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/text-converter.cc
File be/src/exec/text-converter.cc:

http://gerrit.cloudera.org:8080/#/c/16059/8/be/src/exec/text-converter.cc@114
PS8, Line 114:   if (!SupportsCodegenWriteSlot(col_type)) {
             :     stringstream msg;
             :     msg << "TextConverter::CodegenWriteSlot(): "
             :         << col_type
             :         << " isn't supported for CodegenWriteSlot.";
             :     return Status::Expected(msg.str());
             :   }
Can we actually enter this block? If not, then a DCHECK would be better.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id370193af578ecf23ed3c6bfcc65fec448156fa3
Gerrit-Change-Number: 16059
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 17 Jun 2020 15:45:04 +0000
Gerrit-HasComments: Yes

Reply via email to