You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org> on 2016/09/29 00:14:03 UTC
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Bharath Vissapragada has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4557
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
IMPALA-4196: Cross compile bit-byte-functions
Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
---
M be/src/codegen/impala-ir.cc
M be/src/exprs/CMakeLists.txt
R be/src/exprs/bit-byte-functions-ir.cc
3 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/4557/1
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> Yes, could you file a JIRA Bharath?
Thanks. Agree with Michael. Opened IMPALA-4233. I'll go ahead and GVM this.
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
Carry +1
http://gerrit.cloudera.org:8080/#/c/4557/2/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
PS2, Line 2498: built-i
> built-in ?
Done
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4557
to look at the new patch set (#2).
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
IMPALA-4196: Cross compile bit-byte-functions
Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
---
M be/src/codegen/impala-ir.cc
M be/src/exprs/CMakeLists.txt
R be/src/exprs/bit-byte-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 10 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/4557/2
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 2:
Sry missed the test. Added it in PS2.
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 4: Code-Review+2
Rebased.
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> before this fix, did the query fail, or did it just give warnings?
It fails with a warning.
Builtin 'shiftleft' with symbol '_ZN6impala16BitByteFunctions9ShiftLeftEPN10impala_udf15FunctionContextERKNS1_6IntValES6_' does not exist. Verify that all your impalads are the same version.
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
IMPALA-4196: Cross compile bit-byte-functions
Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Reviewed-on: http://gerrit.cloudera.org:8080/4557
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/codegen/impala-ir.cc
M be/src/exprs/CMakeLists.txt
R be/src/exprs/bit-byte-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 10 insertions(+), 1 deletion(-)
Approvals:
Bharath Vissapragada: Looks good to me, approved
Internal Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
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, fn_.name.function_name,
fn_.scalar_fn.symbol);
} else {
DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::NATIVE);
return Status(Substitute("Problem loading UDF '$0':\n$1",
fn_.name.function_name, status.GetDetail()));
return status;
}
}
} else {
// If we got here, either codegen is enabled or we need codegen to run this function.
LlvmCodeGen* codegen;
RETURN_IF_ERROR(state->GetCodegen(&codegen));
if (fn_.binary_type == TFunctionBinaryType::IR) {
string local_path;
RETURN_IF_ERROR(LibCache::instance()->GetLocalLibPath(
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.
RETURN_IF_ERROR(codegen->LinkModule(local_path));
}
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> I think there's another bug in that ScalarFnCall::Prepare() doesn't fallbac
You mean it fails with an *error*, not a warning. Okay.
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Dan Hecht, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4557
to look at the new patch set (#4).
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
IMPALA-4196: Cross compile bit-byte-functions
Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
---
M be/src/codegen/impala-ir.cc
M be/src/exprs/CMakeLists.txt
R be/src/exprs/bit-byte-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 10 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/4557/4
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/2/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
PS2, Line 2498: inbuilt
built-in ?
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 1: -Code-Review
Actually, can you add a regression test for this? There was definitely missing coverage. Maybe in expr-test.cc or test_exprs.py
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> No, it fails with a warning. Basically I tend to agree with Tim that the ex
I think that's a valid point that failure to codegen shouldn't be treated as a fatal error. In general, I think we should rethink on whether codegen should happen in ScalarFnCall()::Prepare(). If possible, we should just push the codegen to happen at the point when GetCodegenedComputeFn() is called the first time like other Expr. It may be slightly more tricky if the external UDF is entirely written in LLVM IR in which case there is no way to fall back to interpretation.
Anyhow, I don't think we need to address all of these in this patch.
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> I think that's a valid point that failure to codegen shouldn't be treated a
Yes, could you file a JIRA Bharath?
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> It fails with a warning.
I think there's another bug in that ScalarFnCall::Prepare() doesn't fallback to interpretation if codegen fails.
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4557/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:
Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
before this fix, did the query fail, or did it just give warnings?
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4557
to look at the new patch set (#3).
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
IMPALA-4196: Cross compile bit-byte-functions
Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
---
M be/src/codegen/impala-ir.cc
M be/src/exprs/CMakeLists.txt
R be/src/exprs/bit-byte-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 10 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/4557/3
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4196: Cross compile bit-byte-functions
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/4557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No