IMPALA-5027: make udf headers buildable externally This fixes a number of issues that prevented building it: * Use of C++11 features * Use of internal headers that are only available in the Impala source tree
Testing: Copied the headers and libImpalaUdf.a to /usr and confirmed I could build impala-udf-samples (with some modifications to udf-test-harness to use some modified arguments to the test harness). We really need a better solution for automatically testing this, e.g. IMPALA-5026, but I wanted to fix the known bugs first. Change-Id: Ibdcdcd96c953cbabc3de04ca66d7aa1774ada99e Reviewed-on: http://gerrit.cloudera.org:8080/6251 Reviewed-by: Matthew Jacobs <[email protected]> Reviewed-by: Dan Hecht <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/5214765e Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/5214765e Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/5214765e Branch: refs/heads/master Commit: 5214765e2ded7e32e75ca463f3b0b18f6c5a2204 Parents: 2644d6a Author: Tim Armstrong <[email protected]> Authored: Fri Mar 3 09:12:01 2017 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Sat Mar 4 22:06:35 2017 +0000 ---------------------------------------------------------------------- be/src/udf/uda-test-harness-impl.h | 10 ++++++++-- be/src/udf/uda-test-harness.h | 3 +++ be/src/udf/udf-debug.h | 3 +++ be/src/udf/udf-test-harness.h | 11 ++++++++--- be/src/udf/udf.h | 33 +++++++++++++++++++++++---------- 5 files changed, 45 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5214765e/be/src/udf/uda-test-harness-impl.h ---------------------------------------------------------------------- diff --git a/be/src/udf/uda-test-harness-impl.h b/be/src/udf/uda-test-harness-impl.h index 05a01a2..c57d0f1 100644 --- a/be/src/udf/uda-test-harness-impl.h +++ b/be/src/udf/uda-test-harness-impl.h @@ -19,10 +19,16 @@ #ifndef IMPALA_UDA_TEST_HARNESS_IMPL_H #define IMPALA_UDA_TEST_HARNESS_IMPL_H +// THIS FILE IS USED BY THE STANDALONE IMPALA UDF DEVELOPMENT KIT. +// IT MUST BE BUILDABLE WITH C++98 AND WITHOUT ANY INTERNAL IMPALA HEADERS. + #include <string> #include <sstream> #include <vector> +// Use boost::shared_ptr instead of std::shared_ptr so it can be built with C++98 +#include <boost/shared_ptr.hpp> + namespace impala_udf { /// Utility class to help test UDAs. This can be used to test the correctness of the @@ -182,7 +188,7 @@ RESULT UdaTestHarnessBase<RESULT, INTERMEDIATE>::ExecuteSingleNode( template<typename RESULT, typename INTERMEDIATE> RESULT UdaTestHarnessBase<RESULT, INTERMEDIATE>::ExecuteOneLevel(int num_nodes, ScopedFunctionContext* result_context) { - std::vector<std::shared_ptr<ScopedFunctionContext>> contexts; + std::vector<boost::shared_ptr<ScopedFunctionContext> > contexts; std::vector<INTERMEDIATE> intermediates; contexts.resize(num_nodes); @@ -238,7 +244,7 @@ RESULT UdaTestHarnessBase<RESULT, INTERMEDIATE>::ExecuteOneLevel(int num_nodes, template<typename RESULT, typename INTERMEDIATE> RESULT UdaTestHarnessBase<RESULT, INTERMEDIATE>::ExecuteTwoLevel( int num1, int num2, ScopedFunctionContext* result_context) { - std::vector<std::shared_ptr<ScopedFunctionContext>> level1_contexts, level2_contexts; + std::vector<boost::shared_ptr<ScopedFunctionContext> > level1_contexts, level2_contexts; std::vector<INTERMEDIATE> level1_intermediates, level2_intermediates; level1_contexts.resize(num1); level2_contexts.resize(num2); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5214765e/be/src/udf/uda-test-harness.h ---------------------------------------------------------------------- diff --git a/be/src/udf/uda-test-harness.h b/be/src/udf/uda-test-harness.h index 7f871cc..75ef9b0 100644 --- a/be/src/udf/uda-test-harness.h +++ b/be/src/udf/uda-test-harness.h @@ -19,6 +19,9 @@ #ifndef IMPALA_UDA_TEST_HARNESS_H #define IMPALA_UDA_TEST_HARNESS_H +// THIS FILE IS USED BY THE STANDALONE IMPALA UDF DEVELOPMENT KIT. +// IT MUST BE BUILDABLE WITH C++98 AND WITHOUT ANY INTERNAL IMPALA HEADERS. + #include <string> #include <sstream> #include <vector> http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5214765e/be/src/udf/udf-debug.h ---------------------------------------------------------------------- diff --git a/be/src/udf/udf-debug.h b/be/src/udf/udf-debug.h index 76e68cc..6c163fa 100644 --- a/be/src/udf/udf-debug.h +++ b/be/src/udf/udf-debug.h @@ -19,6 +19,9 @@ #ifndef IMPALA_UDF_UDF_DEBUG_H #define IMPALA_UDF_UDF_DEBUG_H +// THIS FILE IS USED BY THE STANDALONE IMPALA UDF DEVELOPMENT KIT. +// IT MUST BE BUILDABLE WITH C++98 AND WITHOUT ANY INTERNAL IMPALA HEADERS. + #include "udf/udf.h" #include <string> http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5214765e/be/src/udf/udf-test-harness.h ---------------------------------------------------------------------- diff --git a/be/src/udf/udf-test-harness.h b/be/src/udf/udf-test-harness.h index f730d98..a3cbbdc 100644 --- a/be/src/udf/udf-test-harness.h +++ b/be/src/udf/udf-test-harness.h @@ -19,16 +19,21 @@ #ifndef IMPALA_UDF_TEST_HARNESS_H #define IMPALA_UDF_TEST_HARNESS_H +// THIS FILE IS USED BY THE STANDALONE IMPALA UDF DEVELOPMENT KIT. +// IT MUST BE BUILDABLE WITH C++98 AND WITHOUT ANY INTERNAL IMPALA HEADERS. + #include <iostream> #include <vector> #include <boost/function.hpp> #include <boost/scoped_ptr.hpp> -#include "runtime/runtime-state.h" #include "udf/udf.h" #include "udf/udf-debug.h" -namespace impala { class MemPool; } +namespace impala { + class MemPool; + class RuntimeState; +} namespace impala_udf { @@ -40,7 +45,7 @@ class UdfTestHarness { /// for calling delete on it. This context has additional debugging validation enabled. static FunctionContext* CreateTestContext(const FunctionContext::TypeDesc& return_type, const std::vector<FunctionContext::TypeDesc>& arg_types, - impala::RuntimeState* state = nullptr, impala::MemPool* pool = nullptr); + impala::RuntimeState* state = NULL, impala::MemPool* pool = NULL); /// Use with test contexts to test use of IsArgConstant() and GetConstantArg(). /// constant_args should contain an AnyVal* for each argument of the UDF not including http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5214765e/be/src/udf/udf.h ---------------------------------------------------------------------- diff --git a/be/src/udf/udf.h b/be/src/udf/udf.h index 732f767..7a5328f 100644 --- a/be/src/udf/udf.h +++ b/be/src/udf/udf.h @@ -19,10 +19,22 @@ #ifndef IMPALA_UDF_UDF_H #define IMPALA_UDF_UDF_H +// THIS FILE IS USED BY THE STANDALONE IMPALA UDF DEVELOPMENT KIT. +// IT MUST BE BUILDABLE WITH C++98 AND WITHOUT ANY INTERNAL IMPALA HEADERS. + #include <assert.h> #include <boost/cstdint.hpp> #include <string.h> +// Only use noexcept if the compiler supports C++11 (some system compilers may not +// or may have it disabled by default). +#if __cplusplus >= 201103L +#define NOEXCEPT noexcept +#define USING_TYPE +#else +#define NOEXCEPT +#endif + /// This is the only Impala header required to develop UDFs and UDAs. This header /// contains the types that need to be used and the FunctionContext object. The context /// object serves as the interface object between the UDF/UDA and the impala process. @@ -144,7 +156,7 @@ class FunctionContext { /// If Allocate() fails or causes the memory limit to be exceeded, the error will be /// set in this object causing the query to fail. /// TODO: 'byte_size' should be 64-bit. See IMPALA-2756. - uint8_t* Allocate(int byte_size) noexcept; + uint8_t* Allocate(int byte_size) NOEXCEPT; /// Wrapper around Allocate() to allocate a buffer of the given type "T". template<typename T> @@ -160,10 +172,10 @@ class FunctionContext { /// /// This should be used for buffers that constantly get appended to. /// TODO: 'byte_size' should be 64-bit. See IMPALA-2756. - uint8_t* Reallocate(uint8_t* ptr, int byte_size) noexcept; + uint8_t* Reallocate(uint8_t* ptr, int byte_size) NOEXCEPT; /// Frees a buffer returned from Allocate() or Reallocate() - void Free(uint8_t* buffer) noexcept; + void Free(uint8_t* buffer) NOEXCEPT; /// For allocations that cannot use the Allocate() API provided by this /// object, TrackAllocation()/Free() can be used to just keep count of the @@ -417,7 +429,7 @@ struct BooleanVal : public AnyVal { }; struct TinyIntVal : public AnyVal { - using underlying_type_t = int8_t; + typedef int8_t underlying_type_t; underlying_type_t val; TinyIntVal(underlying_type_t val = 0) : val(val) { } @@ -437,7 +449,7 @@ struct TinyIntVal : public AnyVal { }; struct SmallIntVal : public AnyVal { - using underlying_type_t = int16_t; + typedef int16_t underlying_type_t; underlying_type_t val; SmallIntVal(underlying_type_t val = 0) : val(val) { } @@ -457,7 +469,7 @@ struct SmallIntVal : public AnyVal { }; struct IntVal : public AnyVal { - using underlying_type_t = int32_t; + typedef int32_t underlying_type_t; underlying_type_t val; IntVal(underlying_type_t val = 0) : val(val) { } @@ -477,7 +489,7 @@ struct IntVal : public AnyVal { }; struct BigIntVal : public AnyVal { - using underlying_type_t = int64_t; + typedef int64_t underlying_type_t; underlying_type_t val; BigIntVal(underlying_type_t val = 0) : val(val) { } @@ -589,7 +601,7 @@ struct StringVal : public AnyVal { /// /// The memory backing this StringVal is a local allocation, and so doesn't need /// to be explicitly freed. - StringVal(FunctionContext* context, int len) noexcept; + StringVal(FunctionContext* context, int len) NOEXCEPT; /// Reallocate a StringVal that is backed by a local allocation so that it as /// at least as large as len. May shrink or / expand the string. If the @@ -600,7 +612,7 @@ struct StringVal : public AnyVal { /// local allocation. /// /// Returns true on success, false on failure. - bool Resize(FunctionContext* context, int len) noexcept; + bool Resize(FunctionContext* context, int len) NOEXCEPT; /// Will create a new StringVal with the given dimension and copy the data from the /// parameters. In case of an error will return a NULL string and set an error on the @@ -609,7 +621,8 @@ struct StringVal : public AnyVal { /// Note that the memory for the buffer of the new StringVal is managed by Impala. /// Impala will handle freeing it. Callers should not call Free() on the 'ptr' of /// the StringVal returned. - static StringVal CopyFrom(FunctionContext* ctx, const uint8_t* buf, size_t len) noexcept; + static StringVal CopyFrom(FunctionContext* ctx, const uint8_t* buf, size_t len) + NOEXCEPT; static StringVal null() { StringVal sv;
