Daniel Becker 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 9: (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: llvm::Value* slot_idx_value = codegen->GetI32Constant(slot_idx); : llvm::Function* interpreted_slot_fn = codegen->GetFunction( : IRFunction::HDFS_SCANNER_TEXT_CONVERTER_WRITE_SLOT_INTERPRETED_IR, false); : DCHECK(interpreted_slot_fn != nullptr); : > I don't think that we need to log this. Done 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: : // Codegen is_null_string : bool is_default_null = (len == 2 && null_col_val[0] == '\\' && null_col_val[1] == 'N'); : llvm::Function* is_null_string_fn; : if (is_default_null) { : is_null_string_fn = codegen->GetFunction(IRFunction::IS_NULL_STRING, false); : } > Can we actually enter this block? If not, then a DCHECK would be better. Done. Also added a comment in the header that states that this function should only be called if the type is supported. -- 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: 9 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: Thu, 18 Jun 2020 12:06:17 +0000 Gerrit-HasComments: Yes
