impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Date Fri, 30 Sep 2016 18:14:12 GMT
Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-4196: Cross compile bit-byte-functions

Patch Set 3:

(1 comment)
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:

Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> You mean it fails with an *error*, not a warning.  Okay.
No, it fails with a warning. Basically I tend to agree with Tim that the execution should
fall back to interpretation. The problem seems to be in ScalarFnCall::Prepare() (pasted the
snippet in the bottom) which fails if the codegen path fails for some reason. I think, we
should ideally have something like 

if (skip_codegen || codegen_fails) {
 // Interpret.

Should that be fixed too? Thoughts?

======================== SNIPPET ==================
if (skip_codegen) {
    // Builtins with char arguments must still have <= 8 arguments.
    // TODO: delete when we have codegen for char arguments
    if (has_char_arg_or_result) {
      DCHECK(NumFixedArgs() <= 8 && fn_.binary_type == TFunctionBinaryType::BUILTIN);
    Status status = LibCache::instance()->GetSoFunctionPtr(
        fn_.hdfs_location, fn_.scalar_fn.symbol, &scalar_fn_, &cache_entry_);
    if (!status.ok()) {
      if (fn_.binary_type == TFunctionBinaryType::BUILTIN) {
        // Builtins symbols should exist unless there is a version mismatch.
        return Status(TErrorCode::MISSING_BUILTIN,,
      } else {
        DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::NATIVE);
        return Status(Substitute("Problem loading UDF '$0':\n$1",
  , status.GetDetail()));
        return status;
  } else {
    // If we got here, either codegen is enabled or we need codegen to run this function.
    LlvmCodeGen* codegen;

    if (fn_.binary_type == TFunctionBinaryType::IR) {
      string local_path;
          fn_.hdfs_location, LibCache::TYPE_IR, &local_path));
      // Link the UDF module into this query's main module (essentially copy the UDF
      // module into the main module) so the UDF's functions are available in the main
      // module.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message