IMPALA-3676: Use clang as a static analysis tool This patch adds a script to run clang-tidy over the whole code base. It is a first step towards running clang-tidy over patches as a tool to help users spot bugs before code review.
Because of the number of clang-tidy checks, this patch only addresses some of them. In particular, only checks starting with 'clang' are considered. Many of them which are flaky or not part of our style are excluded from the analysis. This patch also exlcudes some checks which are part of our current style but which would be too laborious to fix over the entire codebase, like using nullptr rather than NULL. This patch also fixes a number of small bugs found by clang-tidy. Finally, this patch adds the class AlignedNew, the purpose of which is to provide correct alignment on heap-allocated data. The global new operator only guarantees 16-byte alignment. A class that includes a member variable that must be aligned on a k-byte boundary for k>16 can inherit from AlignedNew<k> to ensure correct alignment on the heap, quieting clang's -Wover-aligned warning. (Static and stack allocation are required by the standard to respect the alignment of the type and its member variables, so no extra code is needed for allocation in those places.) Change-Id: I4ed168488cb30ddeccd0087f3840541d858f9c06 Reviewed-on: http://gerrit.cloudera.org:8080/4758 Reviewed-by: Jim Apple <[email protected]> Tested-by: Internal 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/14891fe0 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/14891fe0 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/14891fe0 Branch: refs/heads/master Commit: 14891fe00461cc3af579a143811bc59fa6a1b1eb Parents: c03cfe5 Author: Jim Apple <[email protected]> Authored: Fri Oct 7 19:33:28 2016 -0700 Committer: Internal Jenkins <[email protected]> Committed: Fri Nov 4 00:13:12 2016 +0000 ---------------------------------------------------------------------- .clang-tidy | 94 ++++++++++++++++++++ CMakeLists.txt | 3 +- be/CMakeLists.txt | 20 ++++- be/src/benchmarks/bloom-filter-benchmark.cc | 12 ++- be/src/benchmarks/in-predicate-benchmark.cc | 3 +- be/src/benchmarks/parse-timestamp-benchmark.cc | 15 ---- be/src/catalog/catalog-server.cc | 2 +- be/src/catalog/catalog-server.h | 2 +- be/src/codegen/codegen-anyval.cc | 1 - be/src/codegen/impala-ir.cc | 6 ++ be/src/codegen/llvm-codegen.cc | 6 +- be/src/common/compiler-util.h | 3 - be/src/common/init.cc | 2 +- be/src/common/logging.h | 5 -- be/src/common/status.h | 10 +-- be/src/exec/delimited-text-parser.cc | 12 +-- be/src/exec/delimited-text-parser.h | 62 ++++++------- be/src/exec/exec-node.h | 8 +- be/src/exec/external-data-source-executor.h | 2 +- be/src/exec/hash-table-test.cc | 2 +- be/src/exec/hbase-scan-node.h | 2 +- be/src/exec/hdfs-avro-scanner.cc | 5 +- be/src/exec/hdfs-parquet-scanner.h | 4 +- be/src/exec/hdfs-scan-node.cc | 1 - be/src/exec/hdfs-scan-node.h | 5 -- be/src/exec/hdfs-table-writer.h | 2 +- be/src/exec/parquet-column-readers.h | 2 +- be/src/exec/partitioned-hash-join-builder-ir.cc | 4 +- be/src/exec/partitioned-hash-join-builder.cc | 2 + be/src/exec/scanner-context.inline.h | 3 +- be/src/exec/write-stream.h | 2 +- .../experiments/bit-stream-utils.8byte.inline.h | 4 +- be/src/experiments/tuple-splitter-test.cc | 1 - be/src/exprs/aggregate-functions-ir.cc | 30 +++---- be/src/exprs/aggregate-functions.h | 9 +- be/src/exprs/compound-predicates.h | 2 +- be/src/exprs/expr-test.cc | 5 +- be/src/exprs/slot-ref.cc | 3 +- be/src/exprs/string-functions-ir.cc | 6 +- be/src/rpc/TAcceptQueueServer.h | 6 +- be/src/rpc/authentication.cc | 1 + be/src/rpc/authentication.h | 5 +- be/src/rpc/thrift-server.h | 2 + be/src/runtime/buffered-block-mgr.h | 2 +- be/src/runtime/buffered-tuple-stream.cc | 20 ++--- be/src/runtime/buffered-tuple-stream.h | 46 +++++----- be/src/runtime/coordinator.cc | 3 +- be/src/runtime/coordinator.h | 2 +- be/src/runtime/data-stream-sender.cc | 4 +- be/src/runtime/data-stream-sender.h | 1 - be/src/runtime/descriptors.cc | 1 - be/src/runtime/descriptors.h | 9 +- be/src/runtime/exec-env.h | 2 +- be/src/runtime/free-pool-test.cc | 3 +- be/src/runtime/hbase-table.cc | 2 +- be/src/runtime/hdfs-fs-cache.h | 2 +- be/src/runtime/multi-precision-test.cc | 2 +- be/src/runtime/runtime-filter-bank.h | 2 +- be/src/runtime/runtime-filter.h | 2 +- be/src/runtime/scoped-buffer.h | 2 +- be/src/runtime/string-buffer.h | 7 +- be/src/runtime/timestamp-parse-util.cc | 2 +- be/src/runtime/timestamp-value.cc | 4 +- be/src/runtime/tmp-file-mgr.cc | 10 +-- be/src/runtime/tmp-file-mgr.h | 2 +- be/src/scheduling/simple-scheduler-test.cc | 2 +- be/src/service/fe-support.cc | 1 + be/src/service/impala-beeswax-server.cc | 3 +- be/src/service/impala-server.cc | 14 ++- be/src/service/impala-server.h | 12 +-- be/src/service/query-exec-state.h | 2 +- be/src/service/query-result-set.cc | 2 +- be/src/statestore/failure-detector.h | 2 +- be/src/statestore/statestore-subscriber.h | 2 +- be/src/statestore/statestore.cc | 4 +- be/src/statestore/statestore.h | 31 +++---- be/src/testutil/test-udas.cc | 2 +- be/src/testutil/test-udas.h | 2 +- be/src/transport/TSasl.h | 9 +- be/src/transport/TSaslClientTransport.h | 8 +- be/src/transport/TSaslServerTransport.h | 6 +- be/src/transport/TSaslTransport.h | 6 +- be/src/udf/uda-test-harness.h | 6 +- be/src/udf/uda-test.cc | 2 +- be/src/udf/udf-ir.cc | 2 - be/src/udf/udf.cc | 4 +- be/src/udf/udf.h | 2 +- be/src/udf_samples/uda-sample.cc | 2 +- be/src/util/aligned-new.h | 55 ++++++++++++ be/src/util/benchmark-test.cc | 2 +- be/src/util/bit-stream-utils.inline.h | 2 + be/src/util/bit-util-test.cc | 7 +- be/src/util/blocking-queue-test.cc | 2 +- be/src/util/blocking-queue.h | 5 +- be/src/util/bloom-filter-test.cc | 5 +- be/src/util/buffer-builder.h | 22 ++--- be/src/util/compress.cc | 2 +- be/src/util/decompress.cc | 2 +- be/src/util/metrics.h | 6 +- be/src/util/minidump.cc | 10 +-- be/src/util/network-util.cc | 1 + be/src/util/parquet-reader.cc | 4 +- be/src/util/periodic-counter-updater.h | 2 +- be/src/util/pprof-path-handlers.cc | 2 +- be/src/util/progress-updater.cc | 8 +- be/src/util/rle-test.cc | 4 +- be/src/util/runtime-profile.h | 3 +- be/src/util/thread-pool.h | 3 +- be/src/util/webserver.cc | 6 ++ bin/make_impala.sh | 6 +- bin/run_clang_tidy.sh | 54 +++++++++++ buildall.sh | 9 ++ cmake_modules/asan_toolchain.cmake | 44 --------- cmake_modules/clang_toolchain.cmake | 54 +++++++++++ .../org/apache/impala/catalog/BuiltinsDb.java | 20 ++--- 115 files changed, 607 insertions(+), 365 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/.clang-tidy ---------------------------------------------------------------------- diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..5a2bf70 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,94 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +--- +Checks: "-*,clang*,\ +-clang-analyzer-alpha*,\ +-clang-analyzer-core.CallAndMessage,\ +-clang-analyzer-core.NonNullParamChecker,\ +-clang-analyzer-core.NullDereference,\ +-clang-analyzer-core.UndefinedBinaryOperatorResult,\ +-clang-analyzer-core.uninitialized.ArraySubscript,\ +-clang-analyzer-core.uninitialized.Assign,\ +-clang-analyzer-core.uninitialized.Branch,\ +-clang-analyzer-deadcode.DeadStores,\ +-clang-analyzer-unix.Malloc,\ +-clang-analyzer-unix.MallocSizeof,\ +-clang-diagnostic-c++98*,\ +-clang-diagnostic-cast-align,\ +-clang-diagnostic-class-varargs,\ +-clang-diagnostic-conversion,\ +-clang-diagnostic-covered-switch-default,\ +-clang-diagnostic-disabled-macro-expansion,\ +-clang-diagnostic-documentation-html,\ +-clang-diagnostic-documentation-unknown-command,\ +-clang-diagnostic-double-promotion,\ +-clang-diagnostic-exit-time-destructors,\ +-clang-diagnostic-float-conversion,\ +-clang-diagnostic-float-equal,\ +-clang-diagnostic-global-constructors,\ +-clang-diagnostic-gnu-anonymous-struct,\ +-clang-diagnostic-header-hygiene,\ +-clang-diagnostic-implicit-fallthrough,\ +-clang-diagnostic-mismatched-tags,\ +-clang-diagnostic-missing-prototypes,\ +-clang-diagnostic-missing-variable-declarations,\ +-clang-diagnostic-nested-anon-types,\ +-clang-diagnostic-old-style-cast,\ +-clang-diagnostic-overlength-strings,\ +-clang-diagnostic-packed,\ +-clang-diagnostic-padded,\ +-clang-diagnostic-return-type-c-linkage,\ +-clang-diagnostic-shadow,\ +-clang-diagnostic-shorten-64-to-32,\ +-clang-diagnostic-sign-compare,\ +-clang-diagnostic-sign-conversion,\ +-clang-diagnostic-switch-enum,\ +-clang-diagnostic-undefined-reinterpret-cast,\ +-clang-diagnostic-unreachable-code,\ +-clang-diagnostic-unreachable-code-return,\ +-clang-diagnostic-unused-command-line-argument,\ +-clang-diagnostic-unused-local-typedef,\ +-clang-diagnostic-unused-parameter,\ +-clang-diagnostic-used-but-marked-unused,\ +-clang-diagnostic-vla-extension,\ +-clang-diagnostic-weak-vtables" + +# Ignore warnings in gutil + +HeaderFilterRegex: "be/src/\ +(benchmarks\ +|bufferpool\ +|catalog\ +|codegen\ +|common\ +|exec\ +|experiments\ +|exprs\ +|resourcebroker\ +|rpc\ +|runtime\ +|scheduling\ +|service\ +|statestore\ +|testutil\ +|thirdparty\ +|transport\ +|udf\ +|udf_samples\ +|util)" + +AnalyzeTemporaryDtors: true http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/CMakeLists.txt b/CMakeLists.txt index 2e3bf80..dff59ad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -191,7 +191,8 @@ find_package(LlvmBinaries REQUIRED) # Find LLVM libraries to link against. if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG" - OR "${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER") + OR "${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER" + OR "${CMAKE_BUILD_TYPE}" STREQUAL "TIDY") # Use the LLVM libaries with assertions for debug builds. set(LLVM_ROOT ${LLVM_DEBUG_ROOT}) endif() http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt index a44834a..435ab19 100644 --- a/be/CMakeLists.txt +++ b/be/CMakeLists.txt @@ -87,6 +87,21 @@ SET(CXX_FLAGS_RELEASE "${CXX_GCC_FLAGS} -O3 -DNDEBUG -gdwarf-2") SET(CXX_FLAGS_ADDRESS_SANITIZER "${CXX_CLANG_FLAGS} -O1 -g -fsanitize=address -fno-omit-frame-pointer -DADDRESS_SANITIZER") +SET(CXX_FLAGS_TIDY "${CXX_CLANG_FLAGS}") +# Catching unused variables requires an optimization level greater than 0 +SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -O1") +# Ignore assert() and DCHECK() to avoid dead code errors on "DCHECK(false); return +# nullptr" in impossible default switch/case statements. +SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -DNDEBUG") +# Turn all warnings back on. Some will be ignored via .clang-tidy's "Checks" value, but +# this allows different "Checks" settings to be used in different clang-tidy runs without +# recompiling. +SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -Wall -W -Weverything") +# The Tidy build is so verbose (becasue of -Weverything) that it is unlikely to be viewed +# in a terminal and most likely will be redirecto to less, a log file, or /dev/null. In +# those places color codes just make the output harder to read. +SET(CXX_FLAGS_TIDY "${CXX_FLAGS_TIDY} -fno-color-diagnostics") + # Set compile flags based on the build type. if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG") SET(CMAKE_CXX_FLAGS ${CXX_FLAGS_DEBUG}) @@ -94,6 +109,8 @@ elseif ("${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE") SET(CMAKE_CXX_FLAGS ${CXX_FLAGS_RELEASE}) elseif ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER") SET(CMAKE_CXX_FLAGS "${CXX_FLAGS_ADDRESS_SANITIZER}") +elseif ("${CMAKE_BUILD_TYPE}" STREQUAL "TIDY") + SET(CMAKE_CXX_FLAGS "${CXX_FLAGS_TIDY}") else() message(FATAL_ERROR "Unknown build type: ${CMAKE_BUILD_TYPE}") endif() @@ -115,7 +132,8 @@ find_program(CCACHE ccache) if (CCACHE AND NOT DEFINED ENV{DISABLE_CCACHE}) set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ccache) set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ccache) - if ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER") + if ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER" + OR "${CMAKE_BUILD_TYPE}" STREQUAL "TIDY") # Need to set CCACHE_CPP so that ccache calls clang with the original source file for # both preprocessing and compilation. Otherwise, ccache will use clang to preprocess # the file and then call clang with the preprocessed output if not cached. However, http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/benchmarks/bloom-filter-benchmark.cc ---------------------------------------------------------------------- diff --git a/be/src/benchmarks/bloom-filter-benchmark.cc b/be/src/benchmarks/bloom-filter-benchmark.cc index 792be19..c7f529f 100644 --- a/be/src/benchmarks/bloom-filter-benchmark.cc +++ b/be/src/benchmarks/bloom-filter-benchmark.cc @@ -279,7 +279,8 @@ void RunBenchmarks() { Benchmark suite("insert"); vector<unique_ptr<insert::TestData> > testdata; for (int ndv = 10000; ndv <= 100 * 1000 * 1000; ndv *= 100) { - for (double fpp = 0.1; fpp >= 0.001; fpp /= 10) { + for (int log10fpp = -1; log10fpp >= -3; --log10fpp) { + const double fpp = pow(10, log10fpp); testdata.emplace_back(new insert::TestData(BloomFilter::MinLogSpace(ndv, fpp))); snprintf(name, sizeof(name), "ndv %7dk fpp %6.1f%%", ndv/1000, fpp*100); suite.AddBenchmark(name, insert::Benchmark, testdata.back().get()); @@ -292,7 +293,8 @@ void RunBenchmarks() { Benchmark suite("find"); vector<unique_ptr<find::TestData> > testdata; for (int ndv = 10000; ndv <= 100 * 1000 * 1000; ndv *= 100) { - for (double fpp = 0.1; fpp >= 0.001; fpp /= 10) { + for (int log10fpp = -1; log10fpp >= -3; --log10fpp) { + const double fpp = pow(10, log10fpp); testdata.emplace_back( new find::TestData(BloomFilter::MinLogSpace(ndv, fpp), ndv)); snprintf(name, sizeof(name), "present ndv %7dk fpp %6.1f%%", ndv/1000, fpp*100); @@ -309,7 +311,8 @@ void RunBenchmarks() { Benchmark suite("union"); vector<unique_ptr<either::TestData> > testdata; for (int ndv = 10000; ndv <= 100 * 1000 * 1000; ndv *= 100) { - for (double fpp = 0.1; fpp >= 0.001; fpp /= 10) { + for (int log10fpp = -1; log10fpp >= -3; --log10fpp) { + const double fpp = pow(10, log10fpp); testdata.emplace_back( new either::TestData(BloomFilter::MinLogSpace(ndv, fpp))); snprintf(name, sizeof(name), "ndv %7dk fpp %6.1f%%", ndv/1000, fpp*100); @@ -330,7 +333,8 @@ int main(int argc, char **argv) { Benchmark suite("initialize"); vector<unique_ptr<int> > testdata; for (int ndv = 10000; ndv <= 100 * 1000 * 1000; ndv *= 100) { - for (double fpp = 0.1; fpp >= 0.001; fpp /= 10) { + for (int log10fpp = -1; log10fpp >= -3; --log10fpp) { + const double fpp = pow(10, log10fpp); testdata.emplace_back(new int(BloomFilter::MinLogSpace(ndv, fpp))); snprintf(name, sizeof(name), "ndv %7dk fpp %6.1f%%", ndv / 1000, fpp * 100); suite.AddBenchmark(name, initialize::Benchmark, testdata.back().get()); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/benchmarks/in-predicate-benchmark.cc ---------------------------------------------------------------------- diff --git a/be/src/benchmarks/in-predicate-benchmark.cc b/be/src/benchmarks/in-predicate-benchmark.cc index 1392672..1a95f7a 100644 --- a/be/src/benchmarks/in-predicate-benchmark.cc +++ b/be/src/benchmarks/in-predicate-benchmark.cc @@ -161,8 +161,9 @@ #include <gutil/strings/substitute.h> #include "exprs/in-predicate.h" -#include "exprs/in-predicate-ir.cc" +#include "runtime/decimal-value.h" +#include "runtime/string-value.h" #include "udf/udf-test-harness.h" #include "util/benchmark.h" #include "util/cpu-info.h" http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/benchmarks/parse-timestamp-benchmark.cc ---------------------------------------------------------------------- diff --git a/be/src/benchmarks/parse-timestamp-benchmark.cc b/be/src/benchmarks/parse-timestamp-benchmark.cc index 68f4eac..fc9c8d8 100644 --- a/be/src/benchmarks/parse-timestamp-benchmark.cc +++ b/be/src/benchmarks/parse-timestamp-benchmark.cc @@ -58,21 +58,6 @@ using namespace impala; // ImpalaTimeStamp 37.41 83.35X // ImpalaTZTimeStamp 37.39 83.3X -#define VALIDATE 0 - -#if VALIDATE -#define VALIDATE_RESULT(actual, expected, str) \ - if (actual != expected) { \ - cout << "Parse Error. " \ - << "String: " << str \ - << ". Parsed: " << actual << endl; \ - exit(-1); \ - } -#else -#define VALIDATE_RESULT(actual, expected, str) -#endif - - struct TestData { vector<StringValue> data; vector<string> memory; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/catalog/catalog-server.cc ---------------------------------------------------------------------- diff --git a/be/src/catalog/catalog-server.cc b/be/src/catalog/catalog-server.cc index 6f5ba87..099ffde 100644 --- a/be/src/catalog/catalog-server.cc +++ b/be/src/catalog/catalog-server.cc @@ -250,7 +250,7 @@ void CatalogServer::UpdateCatalogTopicCallback( catalog_update_cv_.notify_one(); } -void CatalogServer::GatherCatalogUpdatesThread() { +[[noreturn]] void CatalogServer::GatherCatalogUpdatesThread() { while (1) { unique_lock<mutex> unique_lock(catalog_lock_); // Protect against spurious wakups by checking the value of topic_updates_ready_. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/catalog/catalog-server.h ---------------------------------------------------------------------- diff --git a/be/src/catalog/catalog-server.h b/be/src/catalog/catalog-server.h index 3be92b7..17b8732 100644 --- a/be/src/catalog/catalog-server.h +++ b/be/src/catalog/catalog-server.h @@ -151,7 +151,7 @@ class CatalogServer { /// to get the latest set of catalog objects that exist, along with some metadata on /// each object. The results are stored in the shared catalog_objects_ data structure. /// Also, explicitly releases free memory back to the OS after each complete iteration. - void GatherCatalogUpdatesThread(); + [[noreturn]] void GatherCatalogUpdatesThread(); /// This function determines what items have been added/removed from the catalog /// since the last heartbeat and builds the next topic update to send. To do this, it http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/codegen/codegen-anyval.cc ---------------------------------------------------------------------- diff --git a/be/src/codegen/codegen-anyval.cc b/be/src/codegen/codegen-anyval.cc index 57c08b5..a1ca6d9 100644 --- a/be/src/codegen/codegen-anyval.cc +++ b/be/src/codegen/codegen-anyval.cc @@ -305,7 +305,6 @@ Value* CodegenAnyVal::GetVal(const char* name) { uint32_t idxs[] = {2, 0}; Value* val = builder_->CreateExtractValue(value_, idxs, name); return builder_->CreateTrunc(val, codegen_->GetType(type_), name); - break; } default: DCHECK(false) << "Unsupported type: " << type_; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/codegen/impala-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/codegen/impala-ir.cc b/be/src/codegen/impala-ir.cc index eac17a6..1711bac 100644 --- a/be/src/codegen/impala-ir.cc +++ b/be/src/codegen/impala-ir.cc @@ -21,6 +21,10 @@ // All files here must be added explicitly to the codegen/CMakeLists.txt dependency list #ifdef IR_COMPILE + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wheader-hygiene" + #include "codegen/codegen-anyval-ir.cc" #include "exec/aggregation-node-ir.cc" #include "exec/hash-join-node-ir.cc" @@ -54,6 +58,8 @@ #include "udf/udf-ir.cc" #include "util/hash-util-ir.cc" +#pragma clang diagnostic pop + // Unused function to make sure printf declaration is included in IR module. Used by // LlvmCodegen::CodegenDebugTrace(). void printf_dummy_fn() { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/codegen/llvm-codegen.cc ---------------------------------------------------------------------- diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc index 30cc7d0..f6c2e89 100644 --- a/be/src/codegen/llvm-codegen.cc +++ b/be/src/codegen/llvm-codegen.cc @@ -102,8 +102,8 @@ string LlvmCodeGen::cpu_name_; vector<string> LlvmCodeGen::cpu_attrs_; unordered_set<string> LlvmCodeGen::gv_ref_ir_fns_; -static void LlvmCodegenHandleError(void* user_data, const std::string& reason, - bool gen_crash_diag) { +[[noreturn]] static void LlvmCodegenHandleError( + void* user_data, const std::string& reason, bool gen_crash_diag) { LOG(FATAL) << "LLVM hit fatal error: " << reason.c_str(); } @@ -1416,7 +1416,7 @@ namespace boost { /// Handler for exceptions in cross-compiled functions. /// When boost is configured with BOOST_NO_EXCEPTIONS, it calls this handler instead of /// throwing the exception. -void throw_exception(std::exception const &e) { +[[noreturn]] void throw_exception(std::exception const& e) { LOG(FATAL) << "Cannot handle exceptions in codegen'd code " << e.what(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/common/compiler-util.h ---------------------------------------------------------------------- diff --git a/be/src/common/compiler-util.h b/be/src/common/compiler-util.h index d57767b..80d1112 100644 --- a/be/src/common/compiler-util.h +++ b/be/src/common/compiler-util.h @@ -43,7 +43,4 @@ /// decision, e.g. not inlining a small function on a hot path. #define ALWAYS_INLINE __attribute__((always_inline)) -#define CACHE_ALIGNED __attribute__ ((aligned(64))) - #endif - http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/common/init.cc ---------------------------------------------------------------------- diff --git a/be/src/common/init.cc b/be/src/common/init.cc index b5c7c71..210a642 100644 --- a/be/src/common/init.cc +++ b/be/src/common/init.cc @@ -97,7 +97,7 @@ static scoped_ptr<impala::Thread> maintenance_thread; // time slept. If that exceeds PAUSE_WARN_THRESHOLD_MS, a warning is logged. static scoped_ptr<impala::Thread> pause_monitor; -static void MaintenanceThread() { +[[noreturn]] static void MaintenanceThread() { while (true) { sleep(FLAGS_logbufsecs); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/common/logging.h ---------------------------------------------------------------------- diff --git a/be/src/common/logging.h b/be/src/common/logging.h index 2e96776..03a8294 100644 --- a/be/src/common/logging.h +++ b/be/src/common/logging.h @@ -42,11 +42,6 @@ #define VLOG(level) while(false) std::cout #define VLOG_IS_ON(level) (false) #else - /// GLOG defines this based on the system but doesn't check if it's already - /// been defined. undef it first to avoid warnings. - /// glog MUST be included before gflags. Instead of including them, - /// our files should include this file instead. - #undef _XOPEN_SOURCE #include <glog/logging.h> #include <gflags/gflags.h> #endif http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/common/status.h ---------------------------------------------------------------------- diff --git a/be/src/common/status.h b/be/src/common/status.h index 9a9ea67..a7f31bc 100644 --- a/be/src/common/status.h +++ b/be/src/common/status.h @@ -209,7 +209,7 @@ class Status { /// Returns the error message associated with a non-successful status. const ErrorMsg& msg() const { DCHECK(msg_ != NULL); - return *msg_; + return *msg_; // NOLINT: clang-tidy thinks this might deref a nullptr } /// Add a detail string. Calling this method is only defined on a non-OK message @@ -257,10 +257,10 @@ class Status { }; /// some generally useful macros -#define RETURN_IF_ERROR(stmt) \ - do { \ - Status __status__ = (stmt); \ - if (UNLIKELY(!__status__.ok())) return std::move(__status__); \ +#define RETURN_IF_ERROR(stmt) \ + do { \ + Status __status__ = (stmt); \ + if (UNLIKELY(!__status__.ok())) return __status__; \ } while (false) #define ABORT_IF_ERROR(stmt) \ http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/delimited-text-parser.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/delimited-text-parser.cc b/be/src/exec/delimited-text-parser.cc index 4486fc7..18fcde1 100644 --- a/be/src/exec/delimited-text-parser.cc +++ b/be/src/exec/delimited-text-parser.cc @@ -27,8 +27,13 @@ using namespace impala; DelimitedTextParser::DelimitedTextParser( int num_cols, int num_partition_keys, const bool* is_materialized_col, char tuple_delim, char field_delim, char collection_item_delim, char escape_char) - : num_tuple_delims_(0), + : is_materialized_col_(is_materialized_col), + num_tuple_delims_(0), num_delims_(0), + num_cols_(num_cols), + num_partition_keys_(num_partition_keys), + column_idx_(0), + last_row_delim_offset_(-1), field_delim_(field_delim), process_escapes_(escape_char != '\0'), escape_char_(escape_char), @@ -36,11 +41,6 @@ DelimitedTextParser::DelimitedTextParser( tuple_delim_(tuple_delim), current_column_has_escape_(false), last_char_is_escape_(false), - last_row_delim_offset_(-1), - num_cols_(num_cols), - num_partition_keys_(num_partition_keys), - is_materialized_col_(is_materialized_col), - column_idx_(0), unfinished_tuple_(false){ // Escape character should not be the same as tuple or col delim unless it is the // empty delimiter. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/delimited-text-parser.h ---------------------------------------------------------------------- diff --git a/be/src/exec/delimited-text-parser.h b/be/src/exec/delimited-text-parser.h index 151af8c..b966081 100644 --- a/be/src/exec/delimited-text-parser.h +++ b/be/src/exec/delimited-text-parser.h @@ -164,39 +164,30 @@ class DelimitedTextParser { /// SSE(xmm) register containing the tuple search character(s). __m128i xmm_tuple_search_; - /// The number of delimiters contained in xmm_tuple_search_, i.e. its length. - int num_tuple_delims_; - /// SSE(xmm) register containing the delimiter search character(s). __m128i xmm_delim_search_; - /// The number of delimiters contained in xmm_delim_search_, i.e. its length. - int num_delims_; - /// SSE(xmm) register containing the escape search character. __m128i xmm_escape_search_; - /// Character delimiting fields (to become slots). - char field_delim_; - - /// True if this parser should handle escape characters. - bool process_escapes_; + /// For each col index [0, num_cols_), true if the column should be materialized. + /// Not owned. + const bool* is_materialized_col_; - /// Escape character. Only used if process_escapes_ is true. - char escape_char_; + /// The number of delimiters contained in xmm_tuple_search_, i.e. its length. + int num_tuple_delims_; - /// Character delimiting collection items (to become slots). - char collection_item_delim_; + /// The number of delimiters contained in xmm_delim_search_, i.e. its length. + int num_delims_; - /// Character delimiting tuples. - char tuple_delim_; + /// Number of columns in the table (including partition columns) + int num_cols_; - /// Whether or not the current column has an escape character in it - /// (and needs to be unescaped) - bool current_column_has_escape_; + /// Number of partition columns in the table. + int num_partition_keys_; - /// Whether or not the previous character was the escape character - bool last_char_is_escape_; + /// Index to keep track of the current column in the current file + int column_idx_; /// Used for special processing of \r. /// This will be the offset of the last instance of \r from the end of the @@ -211,18 +202,27 @@ class DelimitedTextParser { uint16_t low_mask_[16]; uint16_t high_mask_[16]; - /// Number of columns in the table (including partition columns) - int num_cols_; + /// Character delimiting fields (to become slots). + char field_delim_; - /// Number of partition columns in the table. - int num_partition_keys_; + /// True if this parser should handle escape characters. + bool process_escapes_; - /// For each col index [0, num_cols_), true if the column should be materialized. - /// Not owned. - const bool* is_materialized_col_; + /// Escape character. Only used if process_escapes_ is true. + char escape_char_; - /// Index to keep track of the current column in the current file - int column_idx_; + /// Character delimiting collection items (to become slots). + char collection_item_delim_; + + /// Character delimiting tuples. + char tuple_delim_; + + /// Whether or not the current column has an escape character in it + /// (and needs to be unescaped) + bool current_column_has_escape_; + + /// Whether or not the previous character was the escape character + bool last_char_is_escape_; /// True if the last tuple is unfinished (not ended with tuple delimiter). bool unfinished_tuple_; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/exec-node.h ---------------------------------------------------------------------- diff --git a/be/src/exec/exec-node.h b/be/src/exec/exec-node.h index 4f3f3fc..d5ca5e3 100644 --- a/be/src/exec/exec-node.h +++ b/be/src/exec/exec-node.h @@ -19,15 +19,15 @@ #ifndef IMPALA_EXEC_EXEC_NODE_H #define IMPALA_EXEC_EXEC_NODE_H -#include <vector> #include <sstream> +#include <vector> #include "common/status.h" #include "exprs/expr-context.h" -#include "runtime/descriptors.h" // for RowDescriptor -#include "util/runtime-profile.h" -#include "util/blocking-queue.h" #include "gen-cpp/PlanNodes_types.h" +#include "runtime/descriptors.h" // for RowDescriptor +#include "util/blocking-queue.h" +#include "util/runtime-profile.h" namespace impala { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/external-data-source-executor.h ---------------------------------------------------------------------- diff --git a/be/src/exec/external-data-source-executor.h b/be/src/exec/external-data-source-executor.h index cdeaf84..dc1ff9b 100644 --- a/be/src/exec/external-data-source-executor.h +++ b/be/src/exec/external-data-source-executor.h @@ -34,7 +34,7 @@ class MetricGroup; /// that the initialization can return an error status if an error occurs. class ExternalDataSourceExecutor { public: - ExternalDataSourceExecutor() : is_initialized_(false), executor_(NULL) { }; + ExternalDataSourceExecutor() : is_initialized_(false), executor_(NULL) { } /// Initialize static JNI state. Called on process startup. static Status InitJNI(MetricGroup* metrics); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/hash-table-test.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/hash-table-test.cc b/be/src/exec/hash-table-test.cc index d903420..9fb5038 100644 --- a/be/src/exec/hash-table-test.cc +++ b/be/src/exec/hash-table-test.cc @@ -475,7 +475,7 @@ class HashTableTest : public testing::Test { EXPECT_EQ(hash_table->EmptyBuckets(), table_size - build_row_val - 1); passes = ht_ctx->EvalAndHashProbe(row); - hash = ht_ctx->expr_values_cache()->CurExprValuesHash(); + (void)ht_ctx->expr_values_cache()->CurExprValuesHash(); EXPECT_TRUE(passes); iter = hash_table->FindProbeRow(ht_ctx.get()); EXPECT_FALSE(iter.AtEnd()); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/hbase-scan-node.h ---------------------------------------------------------------------- diff --git a/be/src/exec/hbase-scan-node.h b/be/src/exec/hbase-scan-node.h index 98a5a50..e0a036d 100644 --- a/be/src/exec/hbase-scan-node.h +++ b/be/src/exec/hbase-scan-node.h @@ -52,7 +52,7 @@ class HBaseScanNode : public ScanNode { /// Close the hbase_scanner_, and report errors. virtual void Close(RuntimeState* state); - const int suggested_max_caching() const { return suggested_max_caching_; } + int suggested_max_caching() const { return suggested_max_caching_; } protected: /// Write debug string of this into out. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/hdfs-avro-scanner.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/hdfs-avro-scanner.cc b/be/src/exec/hdfs-avro-scanner.cc index 5b17fc1..b2e948e 100644 --- a/be/src/exec/hdfs-avro-scanner.cc +++ b/be/src/exec/hdfs-avro-scanner.cc @@ -83,7 +83,7 @@ Status HdfsAvroScanner::Codegen(HdfsScanNodeBase* node, DCHECK(node->runtime_state()->codegen_enabled()); LlvmCodeGen* codegen = node->runtime_state()->codegen(); DCHECK(codegen != NULL); - Function* materialize_tuple_fn; + Function* materialize_tuple_fn = NULL; RETURN_IF_ERROR(CodegenMaterializeTuple(node, codegen, &materialize_tuple_fn)); DCHECK(materialize_tuple_fn != NULL); RETURN_IF_ERROR(CodegenDecodeAvroData(codegen, materialize_tuple_fn, @@ -641,6 +641,7 @@ bool HdfsAvroScanner::MaterializeTuple(const AvroSchemaElement& record_schema, success = MaterializeTuple(element, pool, data, data_end, tuple); break; default: + success = false; DCHECK(false) << "Unsupported SchemaElement: " << type; } if (UNLIKELY(!success)) { @@ -923,7 +924,7 @@ Status HdfsAvroScanner::CodegenReadRecord( // Write read_field_block IR builder->SetInsertPoint(read_field_block); - Value* ret_val; + Value *ret_val = nullptr; if (field->schema->type == AVRO_RECORD) { BasicBlock* insert_before_block = (null_block != NULL) ? null_block : end_field_block; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/hdfs-parquet-scanner.h ---------------------------------------------------------------------- diff --git a/be/src/exec/hdfs-parquet-scanner.h b/be/src/exec/hdfs-parquet-scanner.h index cc5795a..10bc767 100644 --- a/be/src/exec/hdfs-parquet-scanner.h +++ b/be/src/exec/hdfs-parquet-scanner.h @@ -321,7 +321,7 @@ class BoolColumnReader; class HdfsParquetScanner : public HdfsScanner { public: HdfsParquetScanner(HdfsScanNodeBase* scan_node, RuntimeState* state); - virtual ~HdfsParquetScanner() {}; + virtual ~HdfsParquetScanner() {} /// Issue just the footer range for each file. We'll then parse the footer and pick /// out the columns we want. @@ -557,7 +557,7 @@ class HdfsParquetScanner : public HdfsScanner { int row_group_idx, int64_t rows_read); /// Part of the HdfsScanner interface, not used in Parquet. - Status InitNewRange() { return Status::OK(); }; + Status InitNewRange() { return Status::OK(); } /// Transfers the remaining resources backing tuples such as IO buffers and memory /// from mem pools to the given row batch. Closes all column readers. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/hdfs-scan-node.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/hdfs-scan-node.cc b/be/src/exec/hdfs-scan-node.cc index d1092d1..3798107 100644 --- a/be/src/exec/hdfs-scan-node.cc +++ b/be/src/exec/hdfs-scan-node.cc @@ -63,7 +63,6 @@ HdfsScanNode::HdfsScanNode(ObjectPool* pool, const TPlanNode& tnode, done_(false), all_ranges_started_(false), thread_avail_cb_id_(-1), - rm_callback_id_(-1), max_num_scanner_threads_(CpuInfo::num_cores()) { max_materialized_row_batches_ = FLAGS_max_row_batches; if (max_materialized_row_batches_ <= 0) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/hdfs-scan-node.h ---------------------------------------------------------------------- diff --git a/be/src/exec/hdfs-scan-node.h b/be/src/exec/hdfs-scan-node.h index dc85c08..0fc2ba7 100644 --- a/be/src/exec/hdfs-scan-node.h +++ b/be/src/exec/hdfs-scan-node.h @@ -153,11 +153,6 @@ class HdfsScanNode : public HdfsScanNodeBase { /// -1 if no callback is registered. int thread_avail_cb_id_; - /// The id of the callback added to the query resource manager when RM is enabled. - /// Used to remove the callback before this scan node is destroyed. - /// -1 if no callback is registered. - int32_t rm_callback_id_; - /// Maximum number of scanner threads. Set to 'NUM_SCANNER_THREADS' if that query /// option is set. Otherwise, it's set to the number of cpu cores. Scanner threads /// are generally cpu bound so there is no benefit in spinning up more threads than http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/hdfs-table-writer.h ---------------------------------------------------------------------- diff --git a/be/src/exec/hdfs-table-writer.h b/be/src/exec/hdfs-table-writer.h index 5dda484..b3d344f 100644 --- a/be/src/exec/hdfs-table-writer.h +++ b/be/src/exec/hdfs-table-writer.h @@ -86,7 +86,7 @@ class HdfsTableWriter { virtual void Close() = 0; /// Returns the stats for this writer. - TInsertStats& stats() { return stats_; }; + TInsertStats& stats() { return stats_; } /// Default block size to use for this file format. If the file format doesn't /// care, it should return 0 and the hdfs config default will be used. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/parquet-column-readers.h ---------------------------------------------------------------------- diff --git a/be/src/exec/parquet-column-readers.h b/be/src/exec/parquet-column-readers.h index e1a3061..9eefc1a 100644 --- a/be/src/exec/parquet-column-readers.h +++ b/be/src/exec/parquet-column-readers.h @@ -156,7 +156,7 @@ class ParquetColumnReader { /// Returns true if this reader materializes collections (i.e. CollectionValues). virtual bool IsCollectionReader() const { return false; } - const char* filename() const { return parent_->filename(); }; + const char* filename() const { return parent_->filename(); } /// Read the current value (or null) into 'tuple' for this column. This should only be /// called when a value is defined, i.e., def_level() >= http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/partitioned-hash-join-builder-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/partitioned-hash-join-builder-ir.cc b/be/src/exec/partitioned-hash-join-builder-ir.cc index 11974e3..e4785fe 100644 --- a/be/src/exec/partitioned-hash-join-builder-ir.cc +++ b/be/src/exec/partitioned-hash-join-builder-ir.cc @@ -50,7 +50,7 @@ Status PhjBuilder::ProcessBuildBatch( // append it to the null_aware partition. We will need it later. if (UNLIKELY( !AppendRow(null_aware_partition_->build_rows(), build_row, &status))) { - return std::move(status); + return status; } } continue; @@ -71,7 +71,7 @@ Status PhjBuilder::ProcessBuildBatch( const uint32_t partition_idx = hash >> (32 - NUM_PARTITIONING_BITS); Partition* partition = hash_partitions_[partition_idx]; if (UNLIKELY(!AppendRow(partition->build_rows(), build_row, &status))) { - return std::move(status); + return status; } } return Status::OK(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/partitioned-hash-join-builder.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/partitioned-hash-join-builder.cc b/be/src/exec/partitioned-hash-join-builder.cc index 845adbe..8ee8394 100644 --- a/be/src/exec/partitioned-hash-join-builder.cc +++ b/be/src/exec/partitioned-hash-join-builder.cc @@ -17,6 +17,8 @@ #include "exec/partitioned-hash-join-builder.h" +#include <numeric> + #include <gutil/strings/substitute.h> #include "codegen/llvm-codegen.h" http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/scanner-context.inline.h ---------------------------------------------------------------------- diff --git a/be/src/exec/scanner-context.inline.h b/be/src/exec/scanner-context.inline.h index e905db5..5c6f049 100644 --- a/be/src/exec/scanner-context.inline.h +++ b/be/src/exec/scanner-context.inline.h @@ -23,7 +23,7 @@ #include "exec/read-write-util.h" #include "runtime/string-buffer.h" -using namespace impala; +namespace impala { /// Macro to return false if condition is false. Only defined for this file. #define RETURN_IF_FALSE(x) if (UNLIKELY(!(x))) return false @@ -168,4 +168,5 @@ inline bool ScannerContext::Stream::ReadZLong(int64_t* value, Status* status) { #undef RETURN_IF_FALSE +} // namespace impala #endif http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exec/write-stream.h ---------------------------------------------------------------------- diff --git a/be/src/exec/write-stream.h b/be/src/exec/write-stream.h index 1b0cba6..d592a09 100644 --- a/be/src/exec/write-stream.h +++ b/be/src/exec/write-stream.h @@ -32,7 +32,7 @@ namespace impala { class WriteStream { public: - WriteStream() : len_(0) { }; + WriteStream() : len_(0) { } /// Writes bytes to the buffer, returns the number of bytes written inline int WriteBytes(int length, const uint8_t* buf); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/experiments/bit-stream-utils.8byte.inline.h ---------------------------------------------------------------------- diff --git a/be/src/experiments/bit-stream-utils.8byte.inline.h b/be/src/experiments/bit-stream-utils.8byte.inline.h index d0ff243..1b1c05a 100644 --- a/be/src/experiments/bit-stream-utils.8byte.inline.h +++ b/be/src/experiments/bit-stream-utils.8byte.inline.h @@ -98,8 +98,8 @@ inline bool BitReader_8byte::GetValue(int num_bits, T* v) { ++offset_; bit_offset_ -= 64; // Read bits of v that crossed into new byte offset - *v |= BitUtil::TrailingBits( - buffer_[offset_], bit_offset_) << (num_bits - bit_offset_); + *v |= BitUtil::TrailingBits(buffer_[offset_], bit_offset_) + << (num_bits - bit_offset_); } DCHECK_LT(bit_offset_, 64); return true; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/experiments/tuple-splitter-test.cc ---------------------------------------------------------------------- diff --git a/be/src/experiments/tuple-splitter-test.cc b/be/src/experiments/tuple-splitter-test.cc index 5909c3b..df4fc95 100644 --- a/be/src/experiments/tuple-splitter-test.cc +++ b/be/src/experiments/tuple-splitter-test.cc @@ -33,7 +33,6 @@ #define UNUSED_BITS 16 #define USED_BITS 48 -#define UNUSED_UPPER_BITS_MASK 0xFFFF000000000000 #define POINTER_LOWER_BITS_MASK 0x0000FFFFFFFFFFFF #include "common/names.h" http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exprs/aggregate-functions-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/aggregate-functions-ir.cc b/be/src/exprs/aggregate-functions-ir.cc index dd81005..fe19829 100644 --- a/be/src/exprs/aggregate-functions-ir.cc +++ b/be/src/exprs/aggregate-functions-ir.cc @@ -92,7 +92,7 @@ int64_t HllEstimateBias(int64_t estimate) { // Precision index into data arrays // We don't have data for precisions less than 4 DCHECK(impala::AggregateFunctions::HLL_PRECISION >= 4); - const size_t idx = impala::AggregateFunctions::HLL_PRECISION - 4; + static constexpr size_t idx = impala::AggregateFunctions::HLL_PRECISION - 4; // Calculate the square of the difference of this estimate to all // precalculated estimates for a particular precision @@ -170,10 +170,8 @@ StringVal ToStringVal(FunctionContext* context, T val) { // Delimiter to use if the separator is NULL. static const StringVal DEFAULT_STRING_CONCAT_DELIM((uint8_t*)", ", 2); -// Hyperloglog precision. Default taken from paper. Doesn't seem to matter very -// much when between [6,12] -const int AggregateFunctions::HLL_PRECISION = 10; -const int AggregateFunctions::HLL_LEN = 1024; // 2^HLL_PRECISION +constexpr int AggregateFunctions::HLL_PRECISION; +constexpr int AggregateFunctions::HLL_LEN; void AggregateFunctions::InitNull(FunctionContext*, AnyVal* dst) { dst->is_null = true; @@ -983,7 +981,7 @@ void AggregateFunctions::ReservoirSampleUpdate(FunctionContext* ctx, const T& sr } template <typename T> -const StringVal AggregateFunctions::ReservoirSampleSerialize(FunctionContext* ctx, +StringVal AggregateFunctions::ReservoirSampleSerialize(FunctionContext* ctx, const StringVal& src) { if (UNLIKELY(src.is_null)) return src; StringVal result = StringVal::CopyFrom(ctx, src.ptr, src.len); @@ -1779,25 +1777,25 @@ template void AggregateFunctions::ReservoirSampleUpdate( template void AggregateFunctions::ReservoirSampleUpdate( FunctionContext*, const DecimalVal&, StringVal*); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<BooleanVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<BooleanVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<TinyIntVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<TinyIntVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<SmallIntVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<SmallIntVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<IntVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<IntVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<BigIntVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<BigIntVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<FloatVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<FloatVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<DoubleVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<DoubleVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<StringVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<StringVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<TimestampVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<TimestampVal>( FunctionContext*, const StringVal&); -template const StringVal AggregateFunctions::ReservoirSampleSerialize<DecimalVal>( +template StringVal AggregateFunctions::ReservoirSampleSerialize<DecimalVal>( FunctionContext*, const StringVal&); template void AggregateFunctions::ReservoirSampleMerge<BooleanVal>( http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exprs/aggregate-functions.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/aggregate-functions.h b/be/src/exprs/aggregate-functions.h index 76fd2fa..c504468 100644 --- a/be/src/exprs/aggregate-functions.h +++ b/be/src/exprs/aggregate-functions.h @@ -154,7 +154,7 @@ class AggregateFunctions { static void ReservoirSampleMerge(FunctionContext*, const StringVal& src, StringVal* dst); template <typename T> - static const StringVal ReservoirSampleSerialize(FunctionContext*, + static StringVal ReservoirSampleSerialize(FunctionContext*, const StringVal& src); /// Returns 20,000 unsorted samples as a list of comma-separated values. @@ -178,8 +178,11 @@ class AggregateFunctions { /// 1) Hyperloglog: The analysis of a near-optimal cardinality estimation /// algorithm (2007) /// 2) HyperLogLog in Practice (paper from google with some improvements) - static const int HLL_PRECISION; - static const int HLL_LEN; + + /// This precision is the default precision from the paper. It doesn't seem to matter + /// very much when between 6 and 12. + static constexpr int HLL_PRECISION = 10; + static constexpr int HLL_LEN = 1 << HLL_PRECISION; static void HllInit(FunctionContext*, StringVal* slot); template <typename T> static void HllUpdate(FunctionContext*, const T& src, StringVal* dst); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exprs/compound-predicates.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/compound-predicates.h b/be/src/exprs/compound-predicates.h index 0e403ba..cafc30c 100644 --- a/be/src/exprs/compound-predicates.h +++ b/be/src/exprs/compound-predicates.h @@ -30,7 +30,7 @@ namespace impala { class CompoundPredicate: public Predicate { public: static BooleanVal Not(FunctionContext* context, const BooleanVal&); - + protected: CompoundPredicate(const TExprNode& node) : Predicate(node) { } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exprs/expr-test.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc index 0a6b720..5b930e2 100644 --- a/be/src/exprs/expr-test.cc +++ b/be/src/exprs/expr-test.cc @@ -6044,13 +6044,12 @@ TEST_F(ExprTest, BitByteBuiltins) { TestValue("shiftleft(cast(1 as INT), 2)", TYPE_INT, 4); string pow2_30 = lexical_cast<string>(1 << 30); TestValue("shiftleft(" + pow2_30 + ", 2)", TYPE_INT, 0); - TestValue("shiftleft(" + pow2_30 + ", 1)", TYPE_INT, 1 << 31); + TestValue("shiftleft(" + pow2_30 + ", 1)", TYPE_INT, numeric_limits<int32_t>::min()); TestValue("shiftleft(cast(1 as BIGINT), 2)", TYPE_BIGINT, 4); string pow2_62 = lexical_cast<string>(((int64_t)1) << 62); TestValue("shiftleft(" + pow2_62 + ", 2)", TYPE_BIGINT, 0); TestValue("rotateleft(" + pow2_62 + ", 2)", TYPE_BIGINT, 1); - TestValue("shiftleft(" + pow2_62 + ", 1)", TYPE_BIGINT, - ((int64_t)1) << 63); + TestValue("shiftleft(" + pow2_62 + ", 1)", TYPE_BIGINT, numeric_limits<int64_t>::min()); // Basic right shift/rotate tests for all integer types TestValue("shiftright(4, 2)", TYPE_TINYINT, 1); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exprs/slot-ref.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/slot-ref.cc b/be/src/exprs/slot-ref.cc index 0a766fa..eeba42f 100644 --- a/be/src/exprs/slot-ref.cc +++ b/be/src/exprs/slot-ref.cc @@ -17,6 +17,7 @@ #include "exprs/slot-ref.h" +#include <limits> #include <sstream> #include "codegen/codegen-anyval.h" @@ -172,7 +173,7 @@ Status SlotRef::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) // as the join node. When the slot is being used in the scan-node, the tuple is // non-nullable. Used in the join node (and above in the plan tree), it is nullable. // TODO: can we do something better. - const int64_t TUPLE_NULLABLE_MASK = 1L << 63; + constexpr int64_t TUPLE_NULLABLE_MASK = numeric_limits<int64_t>::min(); int64_t unique_slot_id = slot_id_ | ((int64_t)tuple_idx_) << 32; DCHECK_EQ(unique_slot_id & TUPLE_NULLABLE_MASK, 0); if (tuple_is_nullable_) unique_slot_id |= TUPLE_NULLABLE_MASK; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/exprs/string-functions-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/string-functions-ir.cc b/be/src/exprs/string-functions-ir.cc index 39cf898..54efbb3 100644 --- a/be/src/exprs/string-functions-ir.cc +++ b/be/src/exprs/string-functions-ir.cc @@ -670,8 +670,8 @@ void StringFunctions::ParseUrlPrepare( DCHECK_EQ(ctx->GetArgType(1)->type, FunctionContext::TYPE_STRING); StringVal* part = reinterpret_cast<StringVal*>(ctx->GetConstantArg(1)); if (part->is_null) return; - UrlParser::UrlPart* url_part = new UrlParser::UrlPart; - *url_part = UrlParser::GetUrlPart(StringValue::FromStringVal(*part)); + auto url_part = make_unique<UrlParser::UrlPart>( + UrlParser::GetUrlPart(StringValue::FromStringVal(*part))); if (*url_part == UrlParser::INVALID) { stringstream ss; ss << "Invalid URL part: " << AnyValUtil::ToString(*part) << endl @@ -680,7 +680,7 @@ void StringFunctions::ParseUrlPrepare( ctx->SetError(ss.str().c_str()); return; } - ctx->SetFunctionState(scope, url_part); + ctx->SetFunctionState(scope, url_part.release()); } StringVal StringFunctions::ParseUrl( http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/rpc/TAcceptQueueServer.h ---------------------------------------------------------------------- diff --git a/be/src/rpc/TAcceptQueueServer.h b/be/src/rpc/TAcceptQueueServer.h index 9a7a78b..3f5530a 100644 --- a/be/src/rpc/TAcceptQueueServer.h +++ b/be/src/rpc/TAcceptQueueServer.h @@ -19,8 +19,8 @@ // This file was copied from apache::thrift::server::TThreadedServer.cpp v0.9.0, with the // significant changes noted inline below. -#ifndef _THRIFT_SERVER_TACCEPTQUEUESERVER_H_ -#define _THRIFT_SERVER_TACCEPTQUEUESERVER_H_ 1 +#ifndef IMPALA_RPC_TACCEPTQUEUESERVER_H +#define IMPALA_RPC_TACCEPTQUEUESERVER_H #include <thrift/concurrency/Monitor.h> #include <thrift/concurrency/Thread.h> @@ -165,4 +165,4 @@ TAcceptQueueServer::TAcceptQueueServer(const boost::shared_ptr<Processor>& proce } // namespace thrift } // namespace apache -#endif // #ifndef _THRIFT_SERVER_TACCEPTQUEUESERVER_H_ +#endif // #ifndef IMPALA_RPC_TACCEPTQUEUESERVER_H http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/rpc/authentication.cc ---------------------------------------------------------------------- diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc index 4c3ae53..81b745c 100644 --- a/be/src/rpc/authentication.cc +++ b/be/src/rpc/authentication.cc @@ -26,6 +26,7 @@ #include <boost/random/uniform_int.hpp> #include <boost/filesystem.hpp> #include <gutil/strings/substitute.h> +#include <random> #include <string> #include <vector> #include <thrift/Thrift.h> http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/rpc/authentication.h ---------------------------------------------------------------------- diff --git a/be/src/rpc/authentication.h b/be/src/rpc/authentication.h index 3e225bc..858b9ea 100644 --- a/be/src/rpc/authentication.h +++ b/be/src/rpc/authentication.h @@ -28,7 +28,10 @@ #include "transport/TSasl.h" #include "common/status.h" +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wheader-hygiene" using namespace ::apache::thrift::transport; +#pragma clang diagnostic pop namespace impala { @@ -37,7 +40,7 @@ namespace impala { /// servers and clients. class AuthManager { public: - static AuthManager* GetInstance() { return AuthManager::auth_manager_; }; + static AuthManager* GetInstance() { return AuthManager::auth_manager_; } /// Set up internal and external AuthProvider classes. This does a bunch of flag /// checking and calls each AuthProvider->Start(). http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/rpc/thrift-server.h ---------------------------------------------------------------------- diff --git a/be/src/rpc/thrift-server.h b/be/src/rpc/thrift-server.h index 43485f5..5b85134 100644 --- a/be/src/rpc/thrift-server.h +++ b/be/src/rpc/thrift-server.h @@ -83,6 +83,8 @@ class ThriftServer { /// After this callback returns, the memory connection_context references is no longer /// valid and clients must not refer to it again. virtual void ConnectionEnd(const ConnectionContext& connection_context) = 0; + + virtual ~ConnectionHandlerIf() = default; }; static const int DEFAULT_WORKER_THREADS = 2; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/buffered-block-mgr.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/buffered-block-mgr.h b/be/src/runtime/buffered-block-mgr.h index 858a2a9..7b9d487 100644 --- a/be/src/runtime/buffered-block-mgr.h +++ b/be/src/runtime/buffered-block-mgr.h @@ -384,7 +384,7 @@ class BufferedBlockMgr { int num_pinned_buffers(Client* client) const; int num_reserved_buffers_remaining(Client* client) const; - MemTracker* mem_tracker() const { return mem_tracker_.get(); }; + MemTracker* mem_tracker() const { return mem_tracker_.get(); } MemTracker* get_tracker(Client* client) const; int64_t max_block_size() const { return max_block_size_; } int64_t bytes_allocated() const; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/buffered-tuple-stream.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/buffered-tuple-stream.cc b/be/src/runtime/buffered-tuple-stream.cc index 53e1a69..86695f5 100644 --- a/be/src/runtime/buffered-tuple-stream.cc +++ b/be/src/runtime/buffered-tuple-stream.cc @@ -50,12 +50,8 @@ BufferedTupleStream::BufferedTupleStream(RuntimeState* state, const RowDescriptor& row_desc, BufferedBlockMgr* block_mgr, BufferedBlockMgr::Client* client, bool use_initial_small_buffers, bool read_write, const set<SlotId>& ext_varlen_slots) - : use_small_buffers_(use_initial_small_buffers), - delete_on_read_(false), - read_write_(read_write), - state_(state), + : state_(state), desc_(row_desc), - has_nullable_tuple_(row_desc.IsAnyTupleNullable()), block_mgr_(block_mgr), block_mgr_client_(client), total_byte_size_(0), @@ -70,12 +66,16 @@ BufferedTupleStream::BufferedTupleStream(RuntimeState* state, write_block_(NULL), num_pinned_(0), num_small_blocks_(0), - closed_(false), num_rows_(0), - pinned_(true), pin_timer_(NULL), unpin_timer_(NULL), - get_new_block_timer_(NULL) { + get_new_block_timer_(NULL), + read_write_(read_write), + has_nullable_tuple_(row_desc.IsAnyTupleNullable()), + use_small_buffers_(use_initial_small_buffers), + delete_on_read_(false), + closed_(false), + pinned_(true) { read_block_null_indicators_size_ = -1; write_block_null_indicators_size_ = -1; max_null_indicators_size_ = -1; @@ -287,8 +287,8 @@ Status BufferedTupleStream::NewWriteBlock( Status BufferedTupleStream::NewWriteBlockForRow( int64_t row_size, bool* got_block) noexcept { - int64_t block_len; - int64_t null_indicators_size; + int64_t block_len = 0; + int64_t null_indicators_size = 0; if (use_small_buffers_) { *got_block = false; if (blocks_.size() < NUM_SMALL_BLOCKS) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/buffered-tuple-stream.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/buffered-tuple-stream.h b/be/src/runtime/buffered-tuple-stream.h index f741eda..ea5e8b8 100644 --- a/be/src/runtime/buffered-tuple-stream.h +++ b/be/src/runtime/buffered-tuple-stream.h @@ -180,11 +180,11 @@ class BufferedTupleStream { uint64_t block() const { return (data & BLOCK_MASK); - }; + } uint64_t offset() const { return (data & OFFSET_MASK) >> OFFSET_SHIFT; - }; + } uint64_t idx() const { return (data & IDX_MASK) >> IDX_SHIFT; @@ -346,26 +346,12 @@ class BufferedTupleStream { friend class MultiNullableTupleStreamTest_TestComputeRowSize_Test; friend class SimpleTupleStreamTest_TestGetRowsOverflow_Test; - /// If true, this stream is still using small buffers. - bool use_small_buffers_; - - /// If true, blocks are deleted after they are read. - bool delete_on_read_; - - /// If true, read and write operations may be interleaved. Otherwise all calls - /// to AddRow() must occur before calling PrepareForRead() and subsequent calls to - /// GetNext(). - const bool read_write_; - /// Runtime state instance used to check for cancellation. Not owned. RuntimeState* const state_; /// Description of rows stored in the stream. const RowDescriptor& desc_; - /// Whether any tuple in the rows is nullable. - const bool has_nullable_tuple_; - /// Sum of the fixed length portion of all the tuples in desc_. int fixed_tuple_row_size_; @@ -448,22 +434,36 @@ class BufferedTupleStream { /// The total number of small blocks in blocks_; int num_small_blocks_; - bool closed_; // Used for debugging. - /// Number of rows stored in the stream. int64_t num_rows_; + /// Counters added by this object to the parent runtime profile. + RuntimeProfile::Counter* pin_timer_; + RuntimeProfile::Counter* unpin_timer_; + RuntimeProfile::Counter* get_new_block_timer_; + + /// If true, read and write operations may be interleaved. Otherwise all calls + /// to AddRow() must occur before calling PrepareForRead() and subsequent calls to + /// GetNext(). + const bool read_write_; + + /// Whether any tuple in the rows is nullable. + const bool has_nullable_tuple_; + + /// If true, this stream is still using small buffers. + bool use_small_buffers_; + + /// If true, blocks are deleted after they are read. + bool delete_on_read_; + + bool closed_; // Used for debugging. + /// If true, this stream has been explicitly pinned by the caller. This changes the /// memory management of the stream. The blocks are not unpinned until the caller calls /// UnpinAllBlocks(). If false, only the write_block_ and/or read_block_ are pinned /// (both are if read_write_ is true). bool pinned_; - /// Counters added by this object to the parent runtime profile. - RuntimeProfile::Counter* pin_timer_; - RuntimeProfile::Counter* unpin_timer_; - RuntimeProfile::Counter* get_new_block_timer_; - /// The slow path for AddRow() that is called if there is not sufficient space in /// the current block. bool AddRowSlow(TupleRow* row, Status* status) noexcept; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/coordinator.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/coordinator.cc b/be/src/runtime/coordinator.cc index f719e98..c9a166d 100644 --- a/be/src/runtime/coordinator.cc +++ b/be/src/runtime/coordinator.cc @@ -368,7 +368,7 @@ Coordinator::Coordinator(const QuerySchedule& schedule, ExecEnv* exec_env, obj_pool_(new ObjectPool()), query_events_(events), filter_routing_table_complete_(false), - filter_mode_(schedule_.query_options().runtime_filter_mode), + filter_mode_(schedule.query_options().runtime_filter_mode), torn_down_(false) {} Coordinator::~Coordinator() { @@ -1215,7 +1215,6 @@ void Coordinator::InitExecSummary() { } void Coordinator::InitExecProfiles() { - const TQueryExecRequest& request = schedule_.request(); vector<const TPlanFragment*> fragments; schedule_.GetTPlanFragments(&fragments); fragment_profiles_.resize(fragments.size()); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/coordinator.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/coordinator.h b/be/src/runtime/coordinator.h index fab6b29..b6f1aa8 100644 --- a/be/src/runtime/coordinator.h +++ b/be/src/runtime/coordinator.h @@ -100,7 +100,7 @@ struct DebugOptions; /// /// TODO: move into separate subdirectory and move nested classes into separate files /// and unnest them -class Coordinator { +class Coordinator { // NOLINT: The member variables could be re-ordered to save space public: Coordinator(const QuerySchedule& schedule, ExecEnv* exec_env, RuntimeProfile::EventSequence* events); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/data-stream-sender.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/data-stream-sender.cc b/be/src/runtime/data-stream-sender.cc index 2a8ba6a..f80aee5 100644 --- a/be/src/runtime/data-stream-sender.cc +++ b/be/src/runtime/data-stream-sender.cc @@ -32,6 +32,7 @@ #include "runtime/client-cache.h" #include "runtime/mem-tracker.h" #include "runtime/backend-client.h" +#include "util/aligned-new.h" #include "util/debug-util.h" #include "util/network-util.h" #include "util/thread-pool.h" @@ -59,7 +60,7 @@ namespace impala { // at any one time (ie, sending will block if the most recent rpc hasn't finished, // which allows the receiver node to throttle the sender by withholding acks). // *Not* thread-safe. -class DataStreamSender::Channel { +class DataStreamSender::Channel : public CacheLineAligned { public: // Create channel to send data to particular ipaddress/port/query/node // combination. buffer_size is specified in bytes and a soft limit on @@ -330,7 +331,6 @@ DataStreamSender::DataStreamSender(ObjectPool* pool, int sender_id, int per_channel_buffer_size) : DataSink(row_desc), sender_id_(sender_id), - pool_(pool), current_channel_idx_(0), flushed_(false), closed_(false), http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/data-stream-sender.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/data-stream-sender.h b/be/src/runtime/data-stream-sender.h index 9b9e7d0..a8472ad 100644 --- a/be/src/runtime/data-stream-sender.h +++ b/be/src/runtime/data-stream-sender.h @@ -104,7 +104,6 @@ class DataStreamSender : public DataSink { /// Sender instance id, unique within a fragment. int sender_id_; RuntimeState* state_; - ObjectPool* pool_; bool broadcast_; // if true, send all rows on all channels bool random_; // if true, round-robins row batches among channels int current_channel_idx_; // index of current channel to send to if random_ == true http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/descriptors.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/descriptors.cc b/be/src/runtime/descriptors.cc index d106ed4..41485f9 100644 --- a/be/src/runtime/descriptors.cc +++ b/be/src/runtime/descriptors.cc @@ -90,7 +90,6 @@ SlotDescriptor::SlotDescriptor( slot_idx_(tdesc.slotIdx), slot_size_(type_.GetSlotSize()), llvm_field_idx_(-1), - is_null_fn_(NULL), set_not_null_fn_(NULL), set_null_fn_(NULL) { DCHECK_NE(type_.type, TYPE_STRUCT); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/descriptors.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/descriptors.h b/be/src/runtime/descriptors.h index 02193df..8122653 100644 --- a/be/src/runtime/descriptors.h +++ b/be/src/runtime/descriptors.h @@ -19,7 +19,7 @@ #ifndef IMPALA_RUNTIME_DESCRIPTORS_H #define IMPALA_RUNTIME_DESCRIPTORS_H -#include <tr1/unordered_map> +#include <unordered_map> #include <vector> #include <boost/scoped_ptr.hpp> #include <ostream> @@ -164,7 +164,6 @@ class SlotDescriptor { int llvm_field_idx_; /// Cached codegen'd functions - mutable llvm::Function* is_null_fn_; mutable llvm::Function* set_not_null_fn_; mutable llvm::Function* set_null_fn_; @@ -452,9 +451,9 @@ class DescriptorTbl { std::string DebugString() const; private: - typedef std::tr1::unordered_map<TableId, TableDescriptor*> TableDescriptorMap; - typedef std::tr1::unordered_map<TupleId, TupleDescriptor*> TupleDescriptorMap; - typedef std::tr1::unordered_map<SlotId, SlotDescriptor*> SlotDescriptorMap; + typedef std::unordered_map<TableId, TableDescriptor*> TableDescriptorMap; + typedef std::unordered_map<TupleId, TupleDescriptor*> TupleDescriptorMap; + typedef std::unordered_map<SlotId, SlotDescriptor*> SlotDescriptorMap; TableDescriptorMap tbl_desc_map_; TupleDescriptorMap tuple_desc_map_; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/exec-env.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/exec-env.h b/be/src/runtime/exec-env.h index 718c6d0..d4cdf24 100644 --- a/be/src/runtime/exec-env.h +++ b/be/src/runtime/exec-env.h @@ -93,7 +93,7 @@ class ExecEnv { return fragment_exec_thread_pool_.get(); } ImpalaServer* impala_server() { return impala_server_; } - Frontend* frontend() { return frontend_.get(); }; + Frontend* frontend() { return frontend_.get(); } RequestPoolService* request_pool_service() { return request_pool_service_.get(); } CallableThreadPool* rpc_pool() { return async_rpc_pool_.get(); } FragmentMgr* fragment_mgr() { return fragment_mgr_.get(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/free-pool-test.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/free-pool-test.cc b/be/src/runtime/free-pool-test.cc index d808695..f4490f0 100644 --- a/be/src/runtime/free-pool-test.cc +++ b/be/src/runtime/free-pool-test.cc @@ -48,7 +48,7 @@ TEST(FreePoolTest, Basic) { for (int i = 0; i < 10; ++i) { uint8_t* p2 = pool.Allocate(1); *p2 = 111; - ASSERT_EQ(p1, p2); + EXPECT_EQ(p1, p2); EXPECT_EQ(mem_pool.total_allocated_bytes(), 16); pool.Free(p2); } @@ -191,6 +191,7 @@ TEST(FreePoolTest, ReAlloc) { uint8_t* ptr5 = pool.Reallocate(ptr4, 1024); EXPECT_TRUE(ptr4 == ptr5); EXPECT_EQ(mem_pool.total_allocated_bytes(), 1024 + 8 + 2048 + 8 + (1LL << 32) + 8); + pool.Free(ptr5); mem_pool.FreeAll(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/hbase-table.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/hbase-table.cc b/be/src/runtime/hbase-table.cc index bd56c26..14f402f 100644 --- a/be/src/runtime/hbase-table.cc +++ b/be/src/runtime/hbase-table.cc @@ -57,7 +57,7 @@ void HBaseTable::Close(RuntimeState* state) { TErrorCode::GENERAL, "HBaseTable::Close(): Error creating JNIEnv")); } else { env->CallObjectMethod(table_, table_close_id_); - Status s = JniUtil::GetJniExceptionMsg(env, "HBaseTable::Close(): "); + Status s = JniUtil::GetJniExceptionMsg(env, true, "HBaseTable::Close(): "); if (!s.ok()) state->LogError(s.msg()); s = JniUtil::FreeGlobalRef(env, table_); if (!s.ok()) state->LogError(s.msg()); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/hdfs-fs-cache.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/hdfs-fs-cache.h b/be/src/runtime/hdfs-fs-cache.h index fd43431..30b43be 100644 --- a/be/src/runtime/hdfs-fs-cache.h +++ b/be/src/runtime/hdfs-fs-cache.h @@ -79,7 +79,7 @@ class HdfsFsCache { boost::mutex lock_; // protects fs_map_ HdfsFsMap fs_map_; - HdfsFsCache() { }; + HdfsFsCache() { } HdfsFsCache(HdfsFsCache const& l); // disable copy ctor HdfsFsCache& operator=(HdfsFsCache const& l); // disable assignment }; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/multi-precision-test.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/multi-precision-test.cc b/be/src/runtime/multi-precision-test.cc index 9930a8a..babf191 100644 --- a/be/src/runtime/multi-precision-test.cc +++ b/be/src/runtime/multi-precision-test.cc @@ -78,7 +78,7 @@ TEST(MultiPrecisionIntTest, Conversion) { TEST(MultiPrecisionIntTest, HighLowBits) { // x = 0x0f0e0d0c0b0a09080706050403020100 - int128_t x; + int128_t x = 0; for (int i = 0; i < sizeof(x); ++i) { *(reinterpret_cast<uint8_t*>(&x) + i) = i; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/runtime-filter-bank.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/runtime-filter-bank.h b/be/src/runtime/runtime-filter-bank.h index 51408a6..447a70b 100644 --- a/be/src/runtime/runtime-filter-bank.h +++ b/be/src/runtime/runtime-filter-bank.h @@ -100,7 +100,7 @@ class RuntimeFilterBank { BloomFilter* AllocateScratchBloomFilter(int32_t filter_id); /// Default hash seed to use when computing hashed values to insert into filters. - static const int32_t DefaultHashSeed() { return 1234; } + static int32_t DefaultHashSeed() { return 1234; } /// Releases all memory allocated for BloomFilters. void Close(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/runtime-filter.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/runtime-filter.h b/be/src/runtime/runtime-filter.h index be11db9..f64979e 100644 --- a/be/src/runtime/runtime-filter.h +++ b/be/src/runtime/runtime-filter.h @@ -49,7 +49,7 @@ class RuntimeFilter { bool HasBloomFilter() const { return arrival_time_ != 0; } const TRuntimeFilterDesc& filter_desc() const { return filter_desc_; } - const int32_t id() const { return filter_desc().filter_id; } + int32_t id() const { return filter_desc().filter_id; } int64_t filter_size() const { return filter_size_; } /// Sets the internal filter bloom_filter to 'bloom_filter'. Can only legally be called http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/scoped-buffer.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/scoped-buffer.h b/be/src/runtime/scoped-buffer.h index 4841f7f..cf77dd6 100644 --- a/be/src/runtime/scoped-buffer.h +++ b/be/src/runtime/scoped-buffer.h @@ -20,7 +20,7 @@ #include "runtime/mem-tracker.h" -namespace { +namespace impala { /// A scoped memory allocation that is tracked against a MemTracker. /// The allocation is automatically freed when the ScopedBuffer object goes out of scope. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/string-buffer.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/string-buffer.h b/be/src/runtime/string-buffer.h index 7cdf46e..5682bc7 100644 --- a/be/src/runtime/string-buffer.h +++ b/be/src/runtime/string-buffer.h @@ -24,8 +24,6 @@ #include "runtime/mem-tracker.h" #include "runtime/string-value.h" -using namespace strings; - namespace impala { /// Dynamic-sizable string (similar to std::string) but without as many @@ -86,8 +84,9 @@ class StringBuffer { buffer_size_ = std::max<int64_t>(buffer_size_ * 2, new_size); char* new_buffer = reinterpret_cast<char*>(pool_->TryAllocate(buffer_size_)); if (UNLIKELY(new_buffer == NULL)) { - string details = Substitute("StringBuffer failed to grow buffer from $0 " - "to $1 bytes.", old_size, buffer_size_); + string details = + strings::Substitute("StringBuffer failed to grow buffer from $0 to $1 bytes.", + old_size, buffer_size_); return pool_->mem_tracker()->MemLimitExceeded(NULL, details, buffer_size_); } if (LIKELY(len_ > 0)) memcpy(new_buffer, buffer_, len_); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/timestamp-parse-util.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/timestamp-parse-util.cc b/be/src/runtime/timestamp-parse-util.cc index 32d5183..d0c595f 100644 --- a/be/src/runtime/timestamp-parse-util.cc +++ b/be/src/runtime/timestamp-parse-util.cc @@ -339,7 +339,7 @@ bool TimestampParser::Parse(const char* str, int len, const DateTimeFormatContex *d = date(dt_result.year, dt_result.month, dt_result.day); *d += date_duration(day_offset); } - } catch (boost::exception& e) { + } catch (boost::exception&) { is_valid_date = false; } if (!is_valid_date) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/timestamp-value.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/timestamp-value.cc b/be/src/runtime/timestamp-value.cc index 7dfa4c6..72be7fd 100644 --- a/be/src/runtime/timestamp-value.cc +++ b/be/src/runtime/timestamp-value.cc @@ -93,7 +93,7 @@ void TimestampValue::UtcToLocal() { static_cast<unsigned short>(temp.tm_mday)); time_ = time_duration(temp.tm_hour, temp.tm_min, temp.tm_sec, time().fractional_seconds()); - } catch (std::exception& from_boost) { + } catch (std::exception& /* from Boost */) { *this = ptime(not_a_date_time); } } @@ -118,7 +118,7 @@ ptime TimestampValue::UnixTimeToPtime(time_t unix_time) const { } try { return ptime_from_tm(temp_tm); - } catch (std::exception& e) { + } catch (std::exception&) { return ptime(not_a_date_time); } } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/tmp-file-mgr.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/tmp-file-mgr.cc b/be/src/runtime/tmp-file-mgr.cc index b616d18..713cb47 100644 --- a/be/src/runtime/tmp-file-mgr.cc +++ b/be/src/runtime/tmp-file-mgr.cc @@ -137,7 +137,7 @@ Status TmpFileMgr::InitCustom(const vector<string>& tmp_dirs, bool one_dir_per_d } Status TmpFileMgr::NewFile(FileGroup* file_group, const DeviceId& device_id, - const TUniqueId& query_id, File** new_file) { + const TUniqueId& query_id, unique_ptr<File>* new_file) { DCHECK(initialized_); DCHECK_GE(device_id, 0); DCHECK_LT(device_id, tmp_dirs_.size()); @@ -153,7 +153,7 @@ Status TmpFileMgr::NewFile(FileGroup* file_group, const DeviceId& device_id, path new_file_path(tmp_dirs_[device_id].path()); new_file_path /= file_name.str(); - *new_file = new File(this, file_group, device_id, new_file_path.string()); + new_file->reset(new File(this, file_group, device_id, new_file_path.string())); return Status::OK(); } @@ -275,10 +275,10 @@ TmpFileMgr::FileGroup::FileGroup(TmpFileMgr* tmp_file_mgr, int64_t bytes_limit) Status TmpFileMgr::FileGroup::NewFile(const DeviceId& device_id, const TUniqueId& query_id, File** new_file) { - TmpFileMgr::File* tmp_file; + unique_ptr<TmpFileMgr::File> tmp_file; RETURN_IF_ERROR(tmp_file_mgr_->NewFile(this, device_id, query_id, &tmp_file)); - tmp_files_.emplace_back(tmp_file); - if (new_file != NULL) *new_file = tmp_file; + if (new_file != NULL) *new_file = tmp_file.get(); + tmp_files_.emplace_back(std::move(tmp_file)); return Status::OK(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/runtime/tmp-file-mgr.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/tmp-file-mgr.h b/be/src/runtime/tmp-file-mgr.h index bd6c366..2940cf0 100644 --- a/be/src/runtime/tmp-file-mgr.h +++ b/be/src/runtime/tmp-file-mgr.h @@ -192,7 +192,7 @@ class TmpFileMgr { /// responsible for deleting it. The file is not created - creation is deferred until /// the first call to File::AllocateSpace(). Status NewFile(FileGroup* file_group, const DeviceId& device_id, - const TUniqueId& query_id, File** new_file); + const TUniqueId& query_id, std::unique_ptr<File>* new_file); /// Dir stores information about a temporary directory. class Dir { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/scheduling/simple-scheduler-test.cc ---------------------------------------------------------------------- diff --git a/be/src/scheduling/simple-scheduler-test.cc b/be/src/scheduling/simple-scheduler-test.cc index 5743ff2..38638ff 100644 --- a/be/src/scheduling/simple-scheduler-test.cc +++ b/be/src/scheduling/simple-scheduler-test.cc @@ -27,7 +27,7 @@ namespace impala { class SchedulerTest : public testing::Test { protected: - SchedulerTest() { srand(0); }; + SchedulerTest() { srand(0); } }; /// Smoke test to schedule a single table with a single scan range over a single host. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/fe-support.cc ---------------------------------------------------------------------- diff --git a/be/src/service/fe-support.cc b/be/src/service/fe-support.cc index 31aa488..42ceea2 100644 --- a/be/src/service/fe-support.cc +++ b/be/src/service/fe-support.cc @@ -178,6 +178,7 @@ static void ResolveSymbolLookup(const TSymbolLookupParams params, type = LibCache::TYPE_JAR; } else { DCHECK(false) << params.fn_binary_type; + type = LibCache::TYPE_JAR; // Set type to something for the case where DCHECK is off. } // Builtin functions are loaded directly from the running process http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/impala-beeswax-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-beeswax-server.cc b/be/src/service/impala-beeswax-server.cc index 63fa9cd..c3f0c98 100644 --- a/be/src/service/impala-beeswax-server.cc +++ b/be/src/service/impala-beeswax-server.cc @@ -441,7 +441,8 @@ inline void ImpalaServer::QueryHandleToTUniqueId(const QueryHandle& handle, ParseId(handle.id, query_id); } -void ImpalaServer::RaiseBeeswaxException(const string& msg, const char* sql_state) { +[[noreturn]] void ImpalaServer::RaiseBeeswaxException( + const string& msg, const char* sql_state) { BeeswaxException exc; exc.__set_message(msg); exc.__set_SQLState(sql_state); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/impala-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc index c0eed50..c7180fd 100644 --- a/be/src/service/impala-server.cc +++ b/be/src/service/impala-server.cc @@ -207,8 +207,6 @@ const char* ImpalaServer::SQLSTATE_SYNTAX_ERROR_OR_ACCESS_VIOLATION = "42000"; const char* ImpalaServer::SQLSTATE_GENERAL_ERROR = "HY000"; const char* ImpalaServer::SQLSTATE_OPTIONAL_FEATURE_NOT_IMPLEMENTED = "HYC00"; -const int MAX_NM_MISSED_HEARTBEATS = 5; - // Work item for ImpalaServer::cancellation_thread_pool_. class CancellationWork { public: @@ -221,7 +219,7 @@ class CancellationWork { const TUniqueId& query_id() const { return query_id_; } const Status& cause() const { return cause_; } - const bool unregister() const { return unregister_; } + bool unregister() const { return unregister_; } bool operator<(const CancellationWork& other) const { return query_id_ < other.query_id_; @@ -634,14 +632,14 @@ Status ImpalaServer::GetExecSummary(const TUniqueId& query_id, TExecSummary* res return Status::OK(); } -void ImpalaServer::LogFileFlushThread() { +[[noreturn]] void ImpalaServer::LogFileFlushThread() { while (true) { sleep(5); profile_logger_->Flush(); } } -void ImpalaServer::AuditEventLoggerFlushThread() { +[[noreturn]] void ImpalaServer::AuditEventLoggerFlushThread() { while (true) { sleep(5); Status status = audit_event_logger_->Flush(); @@ -655,7 +653,7 @@ void ImpalaServer::AuditEventLoggerFlushThread() { } } -void ImpalaServer::LineageLoggerFlushThread() { +[[noreturn]] void ImpalaServer::LineageLoggerFlushThread() { while (true) { sleep(5); Status status = lineage_logger_->Flush(); @@ -1682,7 +1680,7 @@ void ImpalaServer::RegisterSessionTimeout(int32_t session_timeout) { session_timeout_cv_.notify_one(); } -void ImpalaServer::ExpireSessions() { +[[noreturn]] void ImpalaServer::ExpireSessions() { while (true) { { unique_lock<mutex> timeout_lock(session_timeout_lock_); @@ -1733,7 +1731,7 @@ void ImpalaServer::ExpireSessions() { } } -void ImpalaServer::ExpireQueries() { +[[noreturn]] void ImpalaServer::ExpireQueries() { while (true) { // The following block accomplishes three things: //
