You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jb...@apache.org on 2016/11/04 17:56:07 UTC
[1/5] incubator-impala git commit: IMPALA-3983/IMPALA-3974: Delete
function jar resources after load
Repository: incubator-impala
Updated Branches:
refs/heads/master 1af0aa4ad -> 32294220c
IMPALA-3983/IMPALA-3974: Delete function jar resources after load
The Catalog copies the UDF jar files to the local file system to
load the Java UDF classes for validation purposes. However we
do not clean them up after the UDF load and hence on a deployment
with large number of functions registered, these jar can accumulate
over a period of time and can fill up the tmp space. We fix it by
deleting the jar resource once the function is loaded.
Also, this patch switches to --local_library_dir for copying these
temporary jars instead of using the path from java.io.tmpdir.
Change-Id: I5f9dedb5b342415380c83e61a72eb497371a8199
Reviewed-on: http://gerrit.cloudera.org:8080/4617
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
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/c03cfe51
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c03cfe51
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c03cfe51
Branch: refs/heads/master
Commit: c03cfe51fc6640a4b5d0587582123b56b7575888
Parents: 1af0aa4
Author: Bharath Vissapragada <bh...@cloudera.com>
Authored: Mon Oct 3 14:34:59 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Fri Nov 4 00:07:18 2016 +0000
----------------------------------------------------------------------
be/src/catalog/catalog.cc | 7 +++++--
.../apache/impala/catalog/CatalogServiceCatalog.java | 15 +++++++++------
.../org/apache/impala/common/FileSystemUtil.java | 13 +++++++++++++
.../java/org/apache/impala/service/JniCatalog.java | 6 ++++--
.../impala/testutil/CatalogServiceTestCatalog.java | 3 ++-
tests/custom_cluster/test_permanent_udfs.py | 9 +++++++++
6 files changed, 42 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c03cfe51/be/src/catalog/catalog.cc
----------------------------------------------------------------------
diff --git a/be/src/catalog/catalog.cc b/be/src/catalog/catalog.cc
index 44771c8..9e7af4e 100644
--- a/be/src/catalog/catalog.cc
+++ b/be/src/catalog/catalog.cc
@@ -30,6 +30,7 @@ using namespace impala;
DECLARE_bool(load_auth_to_local_rules);
DECLARE_string(principal);
+DECLARE_string(local_library_dir);
DEFINE_bool(load_catalog_in_background, false,
"If true, loads catalog metadata in the background. If false, metadata is loaded "
@@ -44,7 +45,8 @@ DECLARE_int32(non_impala_java_vlog);
Catalog::Catalog() {
JniMethodDescriptor methods[] = {
- {"<init>", "(ZILjava/lang/String;IIZLjava/lang/String;)V", &catalog_ctor_},
+ {"<init>", "(ZILjava/lang/String;IIZLjava/lang/String;Ljava/lang/String;)V",
+ &catalog_ctor_},
{"updateCatalog", "([B)[B", &update_metastore_id_},
{"execDdl", "([B)[B", &exec_ddl_id_},
{"resetMetadata", "([B)[B", &reset_metadata_id_},
@@ -74,10 +76,11 @@ Catalog::Catalog() {
// and impala is kerberized.
jboolean auth_to_local = FLAGS_load_auth_to_local_rules && !FLAGS_principal.empty();
jstring principal = jni_env->NewStringUTF(FLAGS_principal.c_str());
+ jstring local_library_dir = jni_env->NewStringUTF(FLAGS_local_library_dir.c_str());
jobject catalog = jni_env->NewObject(catalog_class_, catalog_ctor_,
load_in_background, num_metadata_loading_threads, sentry_config,
FlagToTLogLevel(FLAGS_v), FlagToTLogLevel(FLAGS_non_impala_java_vlog),
- auth_to_local, principal);
+ auth_to_local, principal, local_library_dir);
EXIT_IF_EXC(jni_env);
ABORT_IF_ERROR(JniUtil::LocalToGlobalRef(jni_env, catalog, &catalog_));
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c03cfe51/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
index 10f6e4c..f56f502 100644
--- a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
@@ -145,15 +145,15 @@ public class CatalogServiceCatalog extends Catalog {
private final SentryProxy sentryProxy_;
// Local temporary directory to copy UDF Jars.
- private static final String LOCAL_LIBRARY_PATH = new String("file://" +
- System.getProperty("java.io.tmpdir"));
+ private static String localLibraryPath_;
/**
* Initialize the CatalogServiceCatalog. If loadInBackground is true, table metadata
* will be loaded in the background
*/
public CatalogServiceCatalog(boolean loadInBackground, int numLoadingThreads,
- SentryConfig sentryConfig, TUniqueId catalogServiceId, String kerberosPrincipal) {
+ SentryConfig sentryConfig, TUniqueId catalogServiceId, String kerberosPrincipal,
+ String localLibraryPath) {
super(true);
catalogServiceId_ = catalogServiceId;
tableLoadingMgr_ = new TableLoadingMgr(this, numLoadingThreads);
@@ -173,6 +173,7 @@ public class CatalogServiceCatalog extends Catalog {
} else {
sentryProxy_ = null;
}
+ localLibraryPath_ = new String("file://" + localLibraryPath);
}
/**
@@ -428,7 +429,7 @@ public class CatalogServiceCatalog extends Catalog {
/**
* Returns a list of Impala Functions, one per compatible "evaluate" method in the UDF
* class referred to by the given Java function. This method copies the UDF Jar
- * referenced by "function" to a temporary file in "LOCAL_LIBRARY_PATH" and loads it
+ * referenced by "function" to a temporary file in localLibraryPath_ and loads it
* into the jvm. Then we scan all the methods in the class using reflection and extract
* those methods and create corresponding Impala functions. Currently Impala supports
* only "JAR" files for symbols and also a single Jar containing all the dependent
@@ -447,9 +448,9 @@ public class CatalogServiceCatalog extends Catalog {
}
String jarUri = function.getResourceUris().get(0).getUri();
Class<?> udfClass = null;
+ Path localJarPath = null;
try {
- Path localJarPath = new Path(LOCAL_LIBRARY_PATH,
- UUID.randomUUID().toString() + ".jar");
+ localJarPath = new Path(localLibraryPath_, UUID.randomUUID().toString() + ".jar");
try {
FileSystemUtil.copyToLocal(new Path(jarUri), localJarPath);
} catch (IOException e) {
@@ -501,6 +502,8 @@ public class CatalogServiceCatalog extends Catalog {
function.getFunctionName();
LOG.error(errorMsg);
throw new ImpalaRuntimeException(errorMsg, e);
+ } finally {
+ if (localJarPath != null) FileSystemUtil.deleteIfExists(localJarPath);
}
return result;
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c03cfe51/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java b/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
index d9fd6e8..c81e27f 100644
--- a/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
+++ b/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
@@ -377,6 +377,19 @@ public class FileSystemUtil {
}
/**
+ * Delete the file at 'path' if it exists.
+ */
+ public static void deleteIfExists(Path path) {
+ try {
+ FileSystem fs = path.getFileSystem(CONF);
+ if (!fs.exists(path)) return;
+ fs.delete(path);
+ } catch (IOException e) {
+ LOG.warn("Encountered an exception deleting file at path " + path.toString(), e);
+ }
+ }
+
+ /**
* Returns true if the given path is a location which supports caching (e.g. HDFS).
*/
public static boolean isPathCacheable(Path path) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c03cfe51/fe/src/main/java/org/apache/impala/service/JniCatalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/JniCatalog.java b/fe/src/main/java/org/apache/impala/service/JniCatalog.java
index 7d0af54..b35877f 100644
--- a/fe/src/main/java/org/apache/impala/service/JniCatalog.java
+++ b/fe/src/main/java/org/apache/impala/service/JniCatalog.java
@@ -81,7 +81,8 @@ public class JniCatalog {
public JniCatalog(boolean loadInBackground, int numMetadataLoadingThreads,
String sentryServiceConfig, int impalaLogLevel, int otherLogLevel,
- boolean allowAuthToLocal, String kerberosPrincipal) throws InternalException {
+ boolean allowAuthToLocal, String kerberosPrincipal, String localLibraryPath)
+ throws InternalException {
BackendConfig.setAuthToLocal(allowAuthToLocal);
Preconditions.checkArgument(numMetadataLoadingThreads > 0);
// This trick saves having to pass a TLogLevel enum, which is an object and more
@@ -98,7 +99,8 @@ public class JniCatalog {
LOG.info(JniUtil.getJavaVersion());
catalog_ = new CatalogServiceCatalog(loadInBackground,
- numMetadataLoadingThreads, sentryConfig, getServiceId(), kerberosPrincipal);
+ numMetadataLoadingThreads, sentryConfig, getServiceId(), kerberosPrincipal,
+ localLibraryPath);
try {
catalog_.reset();
} catch (CatalogException e) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c03cfe51/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java b/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
index 92b127f..c364ba5 100644
--- a/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
+++ b/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
@@ -31,7 +31,8 @@ public class CatalogServiceTestCatalog extends CatalogServiceCatalog {
public CatalogServiceTestCatalog(boolean loadInBackground, int numLoadingThreads,
SentryConfig sentryConfig, TUniqueId catalogServiceId) {
- super(loadInBackground, numLoadingThreads, sentryConfig, catalogServiceId, null);
+ super(loadInBackground, numLoadingThreads, sentryConfig, catalogServiceId, null,
+ System.getProperty("java.io.tmpdir"));
// Cache pools are typically loaded asynchronously, but as there is no fixed execution
// order for tests, the cache pools are loaded synchronously before the tests are
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c03cfe51/tests/custom_cluster/test_permanent_udfs.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_permanent_udfs.py b/tests/custom_cluster/test_permanent_udfs.py
index c979cd1..0170b8e 100644
--- a/tests/custom_cluster/test_permanent_udfs.py
+++ b/tests/custom_cluster/test_permanent_udfs.py
@@ -15,10 +15,13 @@
# specific language governing permissions and limitations
# under the License.
+import glob
import os
import pytest
+import shutil
import subprocess
+from tempfile import mkdtemp
from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
from tests.common.skip import SkipIfS3, SkipIfIsilon, SkipIfLocal
from tests.common.test_dimensions import create_uncompressed_text_dimension
@@ -33,6 +36,7 @@ class TestUdfPersistence(CustomClusterTestSuite):
HIVE_IMPALA_INTEGRATION_DB = 'hive_impala_integration_db'
HIVE_UDF_JAR = os.getenv('DEFAULT_FS') + '/test-warehouse/hive-exec.jar';
JAVA_UDF_JAR = os.getenv('DEFAULT_FS') + '/test-warehouse/impala-hive-udfs.jar';
+ LOCAL_LIBRARY_DIR = mkdtemp(dir="/tmp")
@classmethod
def get_workload(cls):
@@ -76,6 +80,7 @@ class TestUdfPersistence(CustomClusterTestSuite):
self.client.execute("DROP DATABASE IF EXISTS %s CASCADE" % self.JAVA_FN_TEST_DB)
self.client.execute("DROP DATABASE IF EXISTS %s CASCADE"
% self.HIVE_IMPALA_INTEGRATION_DB)
+ shutil.rmtree(self.LOCAL_LIBRARY_DIR, ignore_errors=True)
def run_stmt_in_hive(self, stmt):
"""
@@ -176,6 +181,8 @@ class TestUdfPersistence(CustomClusterTestSuite):
@SkipIfS3.hive
@SkipIfLocal.hive
@pytest.mark.execute_serially
+ @CustomClusterTestSuite.with_args(
+ catalogd_args= "--local_library_dir=%s" % LOCAL_LIBRARY_DIR)
def test_java_udfs_hive_integration(self):
''' This test checks the integration between Hive and Impala on
CREATE FUNCTION and DROP FUNCTION statements for persistent Java UDFs.
@@ -225,6 +232,8 @@ class TestUdfPersistence(CustomClusterTestSuite):
self.client.execute("INVALIDATE METADATA")
self.verify_function_count(
"SHOW FUNCTIONS in {0}".format(self.HIVE_IMPALA_INTEGRATION_DB), 0)
+ # Make sure we deleted all the temporary jars we copied to the local fs
+ assert len(glob.glob(self.LOCAL_LIBRARY_DIR + "/*.jar")) == 0
@pytest.mark.execute_serially
def test_java_udfs_from_impala(self):
[2/5] incubator-impala git commit: IMPALA-3676: Use clang as a static
analysis tool
Posted by jb...@apache.org.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/impala-server.h
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h
index c40fcb4..b884ea4 100644
--- a/be/src/service/impala-server.h
+++ b/be/src/service/impala-server.h
@@ -409,13 +409,13 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf,
void LogQueryEvents(const QueryExecState& exec_state);
/// Runs once every 5s to flush the profile log file to disk.
- void LogFileFlushThread();
+ [[noreturn]] void LogFileFlushThread();
/// Runs once every 5s to flush the audit log file to disk.
- void AuditEventLoggerFlushThread();
+ [[noreturn]] void AuditEventLoggerFlushThread();
/// Runs once every 5s to flush the lineage log file to disk.
- void LineageLoggerFlushThread();
+ [[noreturn]] void LineageLoggerFlushThread();
/// Copies a query's state into the query log. Called immediately prior to a
/// QueryExecState's deletion. Also writes the query profile to the profile log on disk.
@@ -516,7 +516,7 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf,
void QueryHandleToTUniqueId(const beeswax::QueryHandle& handle, TUniqueId* query_id);
/// Helper function to raise BeeswaxException
- void RaiseBeeswaxException(const std::string& msg, const char* sql_state);
+ [[noreturn]] void RaiseBeeswaxException(const std::string& msg, const char* sql_state);
/// Executes the fetch logic. Doesn't clean up the exec state if an error occurs.
Status FetchInternal(const TUniqueId& query_id, bool start_over,
@@ -598,12 +598,12 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf,
/// sessions for their last-idle times. Those that have been idle for longer than
/// their configured timeout values are 'expired': they will no longer accept queries
/// and any running queries associated with those sessions are unregistered.
- void ExpireSessions();
+ [[noreturn]] void ExpireSessions();
/// Runs forever, walking queries_by_timestamp_ and expiring any queries that have been
/// idle (i.e. no client input and no time spent processing locally) for
/// FLAGS_idle_query_timeout seconds.
- void ExpireQueries();
+ [[noreturn]] void ExpireQueries();
/// Guards query_log_ and query_log_index_
boost::mutex query_log_lock_;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/query-exec-state.h
----------------------------------------------------------------------
diff --git a/be/src/service/query-exec-state.h b/be/src/service/query-exec-state.h
index 78c859c..26dcf4d 100644
--- a/be/src/service/query-exec-state.h
+++ b/be/src/service/query-exec-state.h
@@ -170,7 +170,7 @@ class ImpalaServer::QueryExecState {
}
boost::mutex* lock() { return &lock_; }
boost::mutex* fetch_rows_lock() { return &fetch_rows_lock_; }
- const beeswax::QueryState::type query_state() const { return query_state_; }
+ beeswax::QueryState::type query_state() const { return query_state_; }
const Status& query_status() const { return query_status_; }
void set_result_metadata(const TResultSetMetadata& md) { result_metadata_ = md; }
const RuntimeProfile& profile() const { return profile_; }
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/query-result-set.cc
----------------------------------------------------------------------
diff --git a/be/src/service/query-result-set.cc b/be/src/service/query-result-set.cc
index 3b17af7..8d00af5 100644
--- a/be/src/service/query-result-set.cc
+++ b/be/src/service/query-result-set.cc
@@ -78,7 +78,7 @@ class HS2ColumnarResultSet : public QueryResultSet {
public:
HS2ColumnarResultSet(const TResultSetMetadata& metadata, TRowSet* rowset);
- virtual ~HS2ColumnarResultSet(){};
+ virtual ~HS2ColumnarResultSet() {}
/// Add a row of expr values
virtual Status AddOneRow(const vector<void*>& col_values, const vector<int>& scales);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/failure-detector.h
----------------------------------------------------------------------
diff --git a/be/src/statestore/failure-detector.h b/be/src/statestore/failure-detector.h
index e71faff..c3f504e 100644
--- a/be/src/statestore/failure-detector.h
+++ b/be/src/statestore/failure-detector.h
@@ -77,7 +77,7 @@ class TimeoutFailureDetector : public FailureDetector {
TimeoutFailureDetector(boost::posix_time::time_duration failure_timeout,
boost::posix_time::time_duration suspect_timeout)
: failure_timeout_(failure_timeout),
- suspect_timeout_(suspect_timeout) { };
+ suspect_timeout_(suspect_timeout) { }
virtual PeerState UpdateHeartbeat(const std::string& peer, bool seen);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/statestore-subscriber.h
----------------------------------------------------------------------
diff --git a/be/src/statestore/statestore-subscriber.h b/be/src/statestore/statestore-subscriber.h
index cb40080..1a24a76 100644
--- a/be/src/statestore/statestore-subscriber.h
+++ b/be/src/statestore/statestore-subscriber.h
@@ -263,7 +263,7 @@ class StatestoreSubscriber {
/// During recovery mode, any public methods that are started will block on lock_, which
/// is only released when recovery finishes. In practice, all registrations are made
/// early in the life of an impalad before the statestore could be detected as failed.
- void RecoveryModeChecker();
+ [[noreturn]] void RecoveryModeChecker();
/// Creates a client of the remote statestore and sends a list of
/// topics to register for. Returns OK unless there is some problem
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/statestore.cc
----------------------------------------------------------------------
diff --git a/be/src/statestore/statestore.cc b/be/src/statestore/statestore.cc
index 5ef7cac..e63835f 100644
--- a/be/src/statestore/statestore.cc
+++ b/be/src/statestore/statestore.cc
@@ -199,7 +199,7 @@ void Statestore::Subscriber::AddTransientUpdate(const TopicId& topic_id,
}
}
-const Statestore::TopicEntry::Version Statestore::Subscriber::LastTopicVersionProcessed(
+Statestore::TopicEntry::Version Statestore::Subscriber::LastTopicVersionProcessed(
const TopicId& topic_id) const {
Topics::const_iterator itr = subscribed_topics_.find(topic_id);
return itr == subscribed_topics_.end() ?
@@ -561,7 +561,7 @@ void Statestore::GatherTopicUpdates(const Subscriber& subscriber,
}
}
-const Statestore::TopicEntry::Version Statestore::GetMinSubscriberTopicVersion(
+Statestore::TopicEntry::Version Statestore::GetMinSubscriberTopicVersion(
const TopicId& topic_id, SubscriberId* subscriber_id) {
TopicEntry::Version min_topic_version = numeric_limits<int64_t>::max();
bool found = false;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/statestore.h
----------------------------------------------------------------------
diff --git a/be/src/statestore/statestore.h b/be/src/statestore/statestore.h
index 2bd5237..91adb8d 100644
--- a/be/src/statestore/statestore.h
+++ b/be/src/statestore/statestore.h
@@ -18,31 +18,31 @@
#ifndef STATESTORE_STATESTORE_H
#define STATESTORE_STATESTORE_H
-#include <stdint.h>
+#include <cstdint>
+#include <map>
#include <string>
#include <vector>
-#include <map>
-#include <boost/unordered_map.hpp>
+
#include <boost/scoped_ptr.hpp>
#include <boost/thread/condition_variable.hpp>
+#include <boost/unordered_map.hpp>
#include <boost/uuid/uuid_generators.hpp>
-#include "gen-cpp/Types_types.h"
-#include "gen-cpp/StatestoreSubscriber.h"
#include "gen-cpp/StatestoreService.h"
-#include "util/metrics.h"
-#include "util/collection-metrics.h"
+#include "gen-cpp/StatestoreSubscriber.h"
+#include "gen-cpp/Types_types.h"
#include "rpc/thrift-client.h"
-#include "util/thread-pool.h"
-#include "util/webserver.h"
#include "runtime/client-cache.h"
#include "runtime/timestamp-value.h"
#include "statestore/failure-detector.h"
+#include "util/aligned-new.h"
+#include "util/collection-metrics.h"
+#include "util/metrics.h"
+#include "util/thread-pool.h"
+#include "util/webserver.h"
namespace impala {
-using namespace impala;
-
class Status;
/// The Statestore is a soft-state key-value store that maintains a set of Topics, which
@@ -79,7 +79,7 @@ class Status;
/// processed. The statestore can use this information to send a delta of updates to a
/// subscriber, rather than all items in the topic. For non-delta updates, the statestore
/// will send an update that includes all values in the topic.
-class Statestore {
+class Statestore : public CacheLineAligned {
public:
/// A SubscriberId uniquely identifies a single subscriber, and is
/// provided by the subscriber at registration time.
@@ -251,6 +251,7 @@ class Statestore {
/// Protects access to exit_flag_, but is used mostly to ensure visibility of updates
/// between threads..
boost::mutex exit_flag_lock_;
+
bool exit_flag_;
/// Controls access to topics_. Cannot take subscribers_lock_ after acquiring this lock.
@@ -308,7 +309,7 @@ class Statestore {
/// Returns the last version of the topic which this subscriber has successfully
/// processed. Will never decrease.
- const TopicEntry::Version LastTopicVersionProcessed(const TopicId& topic_id) const;
+ TopicEntry::Version LastTopicVersionProcessed(const TopicId& topic_id) const;
/// Sets the subscriber's last processed version of the topic to the given value. This
/// should only be set when once a subscriber has succesfully processed the given
@@ -484,8 +485,8 @@ class Statestore {
/// TODO: Update the min subscriber version only when a topic is updated, rather than
/// each time a subscriber is updated. One way to do this would be to keep a priority
/// queue in Topic of each subscriber's last processed version of the topic.
- const TopicEntry::Version GetMinSubscriberTopicVersion(const TopicId& topic_id,
- SubscriberId* subscriber_id = NULL);
+ TopicEntry::Version GetMinSubscriberTopicVersion(
+ const TopicId& topic_id, SubscriberId* subscriber_id = NULL);
/// True if the shutdown flag has been set true, false otherwise.
bool ShouldExit();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/testutil/test-udas.cc
----------------------------------------------------------------------
diff --git a/be/src/testutil/test-udas.cc b/be/src/testutil/test-udas.cc
index 844bd33..dc295b5 100644
--- a/be/src/testutil/test-udas.cc
+++ b/be/src/testutil/test-udas.cc
@@ -84,7 +84,7 @@ void MemTestMerge(FunctionContext* context, const BigIntVal& src, BigIntVal* dst
dst->val += src.val;
}
-const BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total) {
+BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total) {
if (total.is_null) return BigIntVal(0);
context->Free(total.val);
return total;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/testutil/test-udas.h
----------------------------------------------------------------------
diff --git a/be/src/testutil/test-udas.h b/be/src/testutil/test-udas.h
index f0d796c..8cd38ec 100644
--- a/be/src/testutil/test-udas.h
+++ b/be/src/testutil/test-udas.h
@@ -25,7 +25,7 @@ using namespace impala_udf;
void MemTestInit(FunctionContext*, BigIntVal* total);
void MemTestUpdate(FunctionContext* context, const BigIntVal& bytes, BigIntVal* total);
void MemTestMerge(FunctionContext* context, const BigIntVal& src, BigIntVal* dst);
-const BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total);
+BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total);
BigIntVal MemTestFinalize(FunctionContext* context, const BigIntVal& total);
#endif
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSasl.h
----------------------------------------------------------------------
diff --git a/be/src/transport/TSasl.h b/be/src/transport/TSasl.h
index c66fcf0..ca13f10 100644
--- a/be/src/transport/TSasl.h
+++ b/be/src/transport/TSasl.h
@@ -18,8 +18,8 @@
* under the License.
*/
-#ifndef _THRIFT_TRANSPORT_TSASL_H_
-#define _THRIFT_TRANSPORT_TSASL_H_ 1
+#ifndef IMPALA_TRANSPORT_TSASL_H
+#define IMPALA_TRANSPORT_TSASL_H
#include <string>
#include <map>
@@ -38,7 +38,10 @@
#include <thrift/transport/TTransportException.h>
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wheader-hygiene"
using namespace apache::thrift::transport;
+#pragma clang diagnostic pop
namespace sasl {
class SaslException : public TTransportException {
@@ -186,4 +189,4 @@ class TSaslServer : public sasl::TSasl {
bool serverStarted;
};
}
-#endif /* _THRIFT_TRANSPORT_TSALS_H_ */
+#endif /* IMPALA_TRANSPORT_TSASL_H */
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSaslClientTransport.h
----------------------------------------------------------------------
diff --git a/be/src/transport/TSaslClientTransport.h b/be/src/transport/TSaslClientTransport.h
index 36dd65d..da708ae 100644
--- a/be/src/transport/TSaslClientTransport.h
+++ b/be/src/transport/TSaslClientTransport.h
@@ -18,8 +18,8 @@
* under the License.
*/
-#ifndef _THRIFT_TRANSPORT_TSSLCLIENTTRANSPORT_H_
-#define _THRIFT_TRANSPORT_TSSLCLIENTTRANSPORT_H_ 1
+#ifndef IMPALA_TRANSPORT_TSSLCLIENTTRANSPORT_H
+#define IMPALA_TRANSPORT_TSSLCLIENTTRANSPORT_H
#include <string>
@@ -54,6 +54,4 @@ class TSaslClientTransport : public TSaslTransport {
}}} // apache::thrift::transport
-#endif // #ifndef _THRIFT_TRANSPORT_TSSLCLIENTTRANSPORT_H_
-
-
+#endif // #ifndef IMPALA_TRANSPORT_TSSLCLIENTTRANSPORT_H
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSaslServerTransport.h
----------------------------------------------------------------------
diff --git a/be/src/transport/TSaslServerTransport.h b/be/src/transport/TSaslServerTransport.h
index 28d17e6..c662771 100644
--- a/be/src/transport/TSaslServerTransport.h
+++ b/be/src/transport/TSaslServerTransport.h
@@ -18,8 +18,8 @@
* under the License.
*/
-#ifndef _THRIFT_TRANSPORT_TSASLSERVERTRANSPORT_H_
-#define _THRIFT_TRANSPORT_TSASLSERVERTRANSPORT_H_ 1
+#ifndef IMPALA_TRANSPORT_TSASLSERVERTRANSPORT_H
+#define IMPALA_TRANSPORT_TSASLSERVERTRANSPORT_H
#include <string>
#include <pthread.h>
@@ -186,4 +186,4 @@ class TSaslServerTransport : public TSaslTransport {
}}} // apache::thrift::transport
-#endif // #ifndef _THRIFT_TRANSPORT_TSSLSERVERTRANSPORT_H_
+#endif // #ifndef IMPALA_TRANSPORT_TSSLSERVERTRANSPORT_H
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSaslTransport.h
----------------------------------------------------------------------
diff --git a/be/src/transport/TSaslTransport.h b/be/src/transport/TSaslTransport.h
index 7dd0fb5..602f089 100644
--- a/be/src/transport/TSaslTransport.h
+++ b/be/src/transport/TSaslTransport.h
@@ -18,8 +18,8 @@
* under the License.
*/
-#ifndef _THRIFT_TRANSPORT_TSSLTRANSPORT_H_
-#define _THRIFT_TRANSPORT_TSSLTRANSPORT_H_ 1
+#ifndef IMPALA_TRANSPORT_TSSLTRANSPORT_H
+#define IMPALA_TRANSPORT_TSSLTRANSPORT_H
#include <string>
@@ -220,4 +220,4 @@ class TSaslTransport : public TVirtualTransport<TSaslTransport> {
}}} // apache::thrift::transport
-#endif // #ifndef _THRIFT_TRANSPORT_TSSLTRANSPORT_H_
+#endif // #ifndef IMPALA_TRANSPORT_TSSLTRANSPORT_H
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/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 b1e3847..7f871cc 100644
--- a/be/src/udf/uda-test-harness.h
+++ b/be/src/udf/uda-test-harness.h
@@ -41,12 +41,14 @@ enum UdaExecutionMode {
template<typename RESULT, typename INTERMEDIATE>
class UdaTestHarnessBase {
public:
+ virtual ~UdaTestHarnessBase() = default;
+
typedef void (*InitFn)(FunctionContext* context, INTERMEDIATE* result);
typedef void (*MergeFn)(FunctionContext* context, const INTERMEDIATE& src,
INTERMEDIATE* dst);
- typedef const INTERMEDIATE (*SerializeFn)(FunctionContext* context,
+ typedef INTERMEDIATE (*SerializeFn)(FunctionContext* context,
const INTERMEDIATE& type);
typedef RESULT (*FinalizeFn)(FunctionContext* context, const INTERMEDIATE& value);
@@ -142,6 +144,8 @@ class UdaTestHarnessBase {
template<typename RESULT, typename INTERMEDIATE, typename INPUT>
class UdaTestHarness : public UdaTestHarnessBase<RESULT, INTERMEDIATE> {
public:
+ virtual ~UdaTestHarness() = default;
+
typedef void (*UpdateFn)(FunctionContext* context, const INPUT& input,
INTERMEDIATE* result);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/uda-test.cc
----------------------------------------------------------------------
diff --git a/be/src/udf/uda-test.cc b/be/src/udf/uda-test.cc
index 6f21699..3ca145f 100644
--- a/be/src/udf/uda-test.cc
+++ b/be/src/udf/uda-test.cc
@@ -116,7 +116,7 @@ void MinUpdate(FunctionContext* context, const StringVal& input, BufferVal* val)
}
// Serialize the state into the min string
-const BufferVal MinSerialize(FunctionContext* context, const BufferVal& intermediate) {
+BufferVal MinSerialize(FunctionContext* context, const BufferVal& intermediate) {
MinState* state = reinterpret_cast<MinState*>(intermediate);
if (state->value == NULL) return intermediate;
// Hack to persist the intermediate state's value without leaking.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/udf-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/udf/udf-ir.cc b/be/src/udf/udf-ir.cc
index aef835b..24773f0 100644
--- a/be/src/udf/udf-ir.cc
+++ b/be/src/udf/udf-ir.cc
@@ -43,10 +43,8 @@ void* FunctionContext::GetFunctionState(FunctionStateScope scope) const {
switch (scope) {
case THREAD_LOCAL:
return impl_->thread_local_fn_state_;
- break;
case FRAGMENT_LOCAL:
return impl_->fragment_local_fn_state_;
- break;
default:
// TODO: signal error somehow
return NULL;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/udf.cc
----------------------------------------------------------------------
diff --git a/be/src/udf/udf.cc b/be/src/udf/udf.cc
index 4ad8d8b..bfdbccc 100644
--- a/be/src/udf/udf.cc
+++ b/be/src/udf/udf.cc
@@ -31,7 +31,7 @@
// on libhdfs.
#include "udf/udf-internal.h"
-#if IMPALA_UDF_SDK_BUILD
+#if defined(IMPALA_UDF_SDK_BUILD) && IMPALA_UDF_SDK_BUILD
// For the SDK build, we are building the .lib that the developers would use to
// write UDFs. They want to link against this to run their UDFs in a test environment.
// Pulling in free-pool is very undesirable since it pulls in many other libraries.
@@ -252,7 +252,7 @@ const char* FunctionContext::effective_user() const {
FunctionContext::UniqueId FunctionContext::query_id() const {
UniqueId id;
-#if IMPALA_UDF_SDK_BUILD
+#if defined(IMPALA_UDF_SDK_BUILD) && IMPALA_UDF_SDK_BUILD
id.hi = id.lo = 0;
#else
id.hi = impl_->state_->query_id().hi;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/udf.h
----------------------------------------------------------------------
diff --git a/be/src/udf/udf.h b/be/src/udf/udf.h
index 5efbd84..e6ec07c 100644
--- a/be/src/udf/udf.h
+++ b/be/src/udf/udf.h
@@ -565,7 +565,7 @@ struct StringVal : public AnyVal {
StringVal(uint8_t* ptr = NULL, int len = 0) : len(len), ptr(ptr) {
assert(len >= 0);
if (ptr == NULL) assert(len == 0);
- };
+ }
/// Construct a StringVal from NULL-terminated c-string. Note: this does not make a
/// copy of ptr so the underlying string must exist as long as this StringVal does.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf_samples/uda-sample.cc
----------------------------------------------------------------------
diff --git a/be/src/udf_samples/uda-sample.cc b/be/src/udf_samples/uda-sample.cc
index 71764f3..23ce807 100644
--- a/be/src/udf_samples/uda-sample.cc
+++ b/be/src/udf_samples/uda-sample.cc
@@ -50,7 +50,7 @@ struct AvgStruct {
};
void AvgInit(FunctionContext* context, BufferVal* val) {
- assert(sizeof(AvgStruct) == 16);
+ static_assert(sizeof(AvgStruct) == 16, "AvgStruct is an unexpected size");
memset(*val, 0, sizeof(AvgStruct));
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/aligned-new.h
----------------------------------------------------------------------
diff --git a/be/src/util/aligned-new.h b/be/src/util/aligned-new.h
new file mode 100644
index 0000000..3a4270c
--- /dev/null
+++ b/be/src/util/aligned-new.h
@@ -0,0 +1,55 @@
+// 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.
+
+#ifndef IMPALA_UTIL_ALIGNED_NEW_H_
+#define IMPALA_UTIL_ALIGNED_NEW_H_
+
+#include <memory>
+
+#include "common/logging.h"
+
+namespace impala {
+
+// Objects that should be allocated, for performance or correctness reasons, at alignment
+// greater than that promised by the global new (16) can inherit publicly from AlignedNew.
+template <size_t ALIGNMENT>
+struct alignas(ALIGNMENT) AlignedNew {
+ static_assert(ALIGNMENT > 0, "ALIGNMENT must be positive");
+ static_assert((ALIGNMENT & (ALIGNMENT - 1)) == 0, "ALIGNMENT must be a power of 2");
+ static_assert(
+ (ALIGNMENT % sizeof(void*)) == 0, "ALIGNMENT must be a multiple of sizeof(void *)");
+ static void* operator new(std::size_t count) { return Allocate(count); }
+ static void* operator new[](std::size_t count) { return Allocate(count); }
+ static void operator delete(void* ptr) { free(ptr); }
+ static void operator delete[](void* ptr) { free(ptr); }
+
+ private:
+ static void* Allocate(std::size_t count) {
+ void* result = nullptr;
+ const auto alloc_failed = posix_memalign(&result, ALIGNMENT, count);
+ if (alloc_failed) {
+ LOG(ERROR) << "Failed to allocate aligned memory; return code " << alloc_failed;
+ throw std::bad_alloc();
+ }
+ return result;
+ }
+};
+
+using CacheLineAligned = AlignedNew<64>;
+}
+
+#endif
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/benchmark-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/benchmark-test.cc b/be/src/util/benchmark-test.cc
index 2c33e3b..fd1ed81 100644
--- a/be/src/util/benchmark-test.cc
+++ b/be/src/util/benchmark-test.cc
@@ -40,7 +40,7 @@ class BenchmarkTest {
public:
static double Measure(Benchmark::BenchmarkFunction fn, void* data) {
return Benchmark::Measure(fn, data);
- };
+ }
};
void TestFunction(int batch_size, void* d) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/bit-stream-utils.inline.h
----------------------------------------------------------------------
diff --git a/be/src/util/bit-stream-utils.inline.h b/be/src/util/bit-stream-utils.inline.h
index 41648e3..dee2456 100644
--- a/be/src/util/bit-stream-utils.inline.h
+++ b/be/src/util/bit-stream-utils.inline.h
@@ -93,6 +93,8 @@ inline bool BitReader::GetValue(int num_bits, T* v) {
if (UNLIKELY(byte_offset_ * 8 + bit_offset_ + num_bits > max_bytes_ * 8)) return false;
+ DCHECK_GE(bit_offset_, 0);
+ DCHECK_LE(bit_offset_, 64);
*v = BitUtil::TrailingBits(buffered_values_, bit_offset_ + num_bits) >> bit_offset_;
bit_offset_ += num_bits;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/bit-util-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/bit-util-test.cc b/be/src/util/bit-util-test.cc
index 0c0db8b..663f10f 100644
--- a/be/src/util/bit-util-test.cc
+++ b/be/src/util/bit-util-test.cc
@@ -90,9 +90,9 @@ TEST(BitUtil, TrailingBits) {
BOOST_BINARY(1 1 1 1 1 1 1 1));
EXPECT_EQ(BitUtil::TrailingBits(0, 1), 0);
EXPECT_EQ(BitUtil::TrailingBits(0, 64), 0);
- EXPECT_EQ(BitUtil::TrailingBits(1LL << 63, 0), 0);
- EXPECT_EQ(BitUtil::TrailingBits(1LL << 63, 63), 0);
- EXPECT_EQ(BitUtil::TrailingBits(1LL << 63, 64), 1LL << 63);
+ EXPECT_EQ(BitUtil::TrailingBits(1ULL << 63, 0), 0);
+ EXPECT_EQ(BitUtil::TrailingBits(1ULL << 63, 63), 0);
+ EXPECT_EQ(BitUtil::TrailingBits(1ULL << 63, 64), 1ULL << 63);
}
// Test different SIMD functionality units with an input/output buffer.
@@ -141,6 +141,7 @@ void TestByteSwapSimd_Unit(const int64_t CpuFlag) {
// CpuFlag == 0 for BitUtil::ByteSwap;
// buf_size parameter indicates the size of input/output buffer.
void TestByteSwapSimd(const int64_t CpuFlag, const int buf_size) {
+ if (buf_size <= 0) return;
uint8_t src_buf[buf_size];
uint8_t dst_buf[buf_size];
std::iota(src_buf, src_buf + buf_size, 0);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/blocking-queue-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/blocking-queue-test.cc b/be/src/util/blocking-queue-test.cc
index f45853a..f61de21 100644
--- a/be/src/util/blocking-queue-test.cc
+++ b/be/src/util/blocking-queue-test.cc
@@ -67,7 +67,7 @@ TEST(BlockingQueueTest, TestPutWithTimeout) {
ASSERT_TRUE(test_queue.BlockingPutWithTimeout(3, timeout_micros));
}
-class MultiThreadTest {
+class MultiThreadTest { // NOLINT: members are not arranged for minimal padding
public:
MultiThreadTest()
: iterations_(10000),
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/blocking-queue.h
----------------------------------------------------------------------
diff --git a/be/src/util/blocking-queue.h b/be/src/util/blocking-queue.h
index e12793e..a762d5d 100644
--- a/be/src/util/blocking-queue.h
+++ b/be/src/util/blocking-queue.h
@@ -27,6 +27,7 @@
#include "common/atomic.h"
#include "common/compiler-util.h"
+#include "util/aligned-new.h"
#include "util/condition-variable.h"
#include "util/stopwatch.h"
#include "util/time.h"
@@ -43,7 +44,7 @@ namespace impala {
/// will atomically swap the 'put_list_' with 'get_list_'. The swapping happens with both
/// the 'get_lock_' and 'put_lock_' held.
template <typename T>
-class BlockingQueue {
+class BlockingQueue : public CacheLineAligned {
public:
BlockingQueue(size_t max_elements)
: shutdown_(false),
@@ -233,7 +234,7 @@ class BlockingQueue {
/// variable doesn't include the time which other threads block waiting for 'get_lock_'.
int64_t total_get_wait_time_;
-} CACHE_ALIGNED;
+};
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/bloom-filter-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/bloom-filter-test.cc b/be/src/util/bloom-filter-test.cc
index 9e63227..20c2655 100644
--- a/be/src/util/bloom-filter-test.cc
+++ b/be/src/util/bloom-filter-test.cc
@@ -180,7 +180,8 @@ TEST(BloomFilter, FindInvalid) {
// Test that MaxNdv() and MinLogSpace() are dual
TEST(BloomFilter, MinSpaceMaxNdv) {
- for (double fpp = 0.25; fpp >= 1.0 / (1ull << 63); fpp /= 2) {
+ for (int log2fpp = -2; log2fpp >= -63; --log2fpp) {
+ const double fpp = pow(2, log2fpp);
for (int given_log_space = 8; given_log_space < 30; ++given_log_space) {
const size_t derived_ndv = BloomFilter::MaxNdv(given_log_space, fpp);
int derived_log_space = BloomFilter::MinLogSpace(derived_ndv, fpp);
@@ -225,7 +226,7 @@ TEST(BloomFilter, MinSpaceEdgeCase) {
// Check that MinLogSpace() and FalsePositiveProb() are dual
TEST(BloomFilter, MinSpaceForFpp) {
for (size_t ndv = 10000; ndv < 100 * 1000 * 1000; ndv *= 1.01) {
- for (double fpp = 0.1; fpp > pow(2, -20); fpp *= 0.99) {
+ for (double fpp = 0.1; fpp > pow(2, -20); fpp *= 0.99) { // NOLINT: loop on double
// When contructing a Bloom filter, we can request a particular fpp by calling
// MinLogSpace().
const int min_log_space = BloomFilter::MinLogSpace(ndv, fpp);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/buffer-builder.h
----------------------------------------------------------------------
diff --git a/be/src/util/buffer-builder.h b/be/src/util/buffer-builder.h
index afcf9ae..a421e20 100644
--- a/be/src/util/buffer-builder.h
+++ b/be/src/util/buffer-builder.h
@@ -27,35 +27,31 @@ namespace impala {
/// Utility class to build an in-memory buffer.
class BufferBuilder {
public:
- BufferBuilder(uint8_t* dst_buffer, int dst_len)
- : buffer_(dst_buffer), capacity_(dst_len), size_(0) {
- }
-
- BufferBuilder(char* dst_buffer, int dst_len)
- : buffer_(reinterpret_cast<uint8_t*>(dst_buffer)),
- capacity_(dst_len), size_(0) {
- }
+ BufferBuilder(uint8_t* dst_buffer, int dst_len)
+ : buffer_(dst_buffer), capacity_(dst_len), size_(0) {}
+
+ BufferBuilder(char* dst_buffer, int dst_len)
+ : buffer_(reinterpret_cast<uint8_t*>(dst_buffer)), capacity_(dst_len), size_(0) {}
- inline void Append(const void* buffer, int len) {
+ inline void Append(const void* buffer, int len) __attribute__((nonnull)) {
DCHECK_LE(size_ + len, capacity_);
memcpy(buffer_ + size_, buffer, len);
size_ += len;
}
- template<typename T>
+ template <typename T>
inline void Append(const T& v) {
Append(&v, sizeof(T));
}
int capacity() const { return capacity_; }
int size() const { return size_; }
-
- private:
+
+ private:
uint8_t* buffer_;
int capacity_;
int size_;
};
-
}
#endif
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/compress.cc
----------------------------------------------------------------------
diff --git a/be/src/util/compress.cc b/be/src/util/compress.cc
index 7c97737..3737828 100644
--- a/be/src/util/compress.cc
+++ b/be/src/util/compress.cc
@@ -158,7 +158,7 @@ Status BzipCompressor::ProcessBlock(bool output_preallocated, int64_t input_leng
out_buffer_ = temp_memory_pool_->Allocate(buffer_length_);
}
- unsigned int outlen;
+ unsigned int outlen = static_cast<unsigned int>(buffer_length_);
int ret = BZ_OUTBUFF_FULL;
while (ret == BZ_OUTBUFF_FULL) {
if (out_buffer_ == NULL) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/decompress.cc
----------------------------------------------------------------------
diff --git a/be/src/util/decompress.cc b/be/src/util/decompress.cc
index edb5490..7896c33 100644
--- a/be/src/util/decompress.cc
+++ b/be/src/util/decompress.cc
@@ -279,7 +279,7 @@ Status BzipDecompressor::ProcessBlock(bool output_preallocated, int64_t input_le
}
int ret = BZ_OUTBUFF_FULL;
- unsigned int outlen;
+ unsigned int outlen = static_cast<unsigned int>(buffer_length_);
// TODO: IMPALA-3073 Verify if compressed block could be multistream. If yes, we need
// to support it and shouldn't stop decompressing while ret == BZ_STREAM_END.
while (ret == BZ_OUTBUFF_FULL) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/metrics.h
----------------------------------------------------------------------
diff --git a/be/src/util/metrics.h b/be/src/util/metrics.h
index 8b3942d..93497ea 100644
--- a/be/src/util/metrics.h
+++ b/be/src/util/metrics.h
@@ -60,7 +60,7 @@ class MetricDefs {
/// Contains the map of all TMetricDefs, non-const for testing
MetricDefsConstants metric_defs_;
- MetricDefs() { };
+ MetricDefs() { }
DISALLOW_COPY_AND_ASSIGN(MetricDefs);
};
@@ -190,8 +190,8 @@ class SimpleMetric : public Metric {
document->AddMember(key_.c_str(), val, document->GetAllocator());
}
- const TUnit::type unit() const { return unit_; }
- const TMetricKind::type kind() const { return metric_kind; }
+ TUnit::type unit() const { return unit_; }
+ TMetricKind::type kind() const { return metric_kind; }
protected:
/// Called to compute value_ if necessary during calls to value(). The more natural
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/minidump.cc
----------------------------------------------------------------------
diff --git a/be/src/util/minidump.cc b/be/src/util/minidump.cc
index 4567c48..b87ea90 100644
--- a/be/src/util/minidump.cc
+++ b/be/src/util/minidump.cc
@@ -29,6 +29,7 @@
#include <iomanip>
#include <map>
#include <signal.h>
+#include <sstream>
#include "common/logging.h"
#include "common/version.h"
@@ -41,15 +42,12 @@ using boost::filesystem::create_directories;
using boost::filesystem::is_regular_file;
using boost::filesystem::path;
using boost::filesystem::remove;
-using boost::system::error_code;
DECLARE_string(log_dir);
DECLARE_string(minidump_path);
DECLARE_int32(max_minidumps);
DECLARE_int32(minidump_size_limit_hint_kb);
-#define MINIDUMP_LOG_BUF_SIZE 256
-
namespace impala {
/// Breakpad ExceptionHandler. It registers its own signal handlers to write minidump
@@ -127,7 +125,7 @@ static void CheckAndRemoveMinidumps(int max_minidumps) {
glob(pattern.c_str(), GLOB_TILDE, NULL, &result);
for (size_t i = 0; i < result.gl_pathc; ++i) {
const path minidump_path(result.gl_pathv[i]);
- error_code err;
+ boost::system::error_code err;
bool is_file = is_regular_file(minidump_path, err);
// is_regular_file() calls stat() eventually, which can return errors, e.g. if the
// file permissions prevented access or the path was wrong (see 'man 2 stat' for
@@ -178,7 +176,7 @@ static void CheckAndRemoveMinidumps(int max_minidumps) {
DCHECK_GT(files_to_delete, 0);
auto to_delete = timestamp_to_path.begin();
for (int i = 0; i < files_to_delete; ++i, ++to_delete) {
- error_code err;
+ boost::system::error_code err;
remove(to_delete->second, err);
if (!err) {
LOG(INFO) << "Removed old minidump file : " << to_delete->second;
@@ -209,7 +207,7 @@ Status RegisterMinidump(const char* cmd_line_path) {
// Create the directory if it is not there. The minidump doesn't get written if there is
// no directory.
- error_code err;
+ boost::system::error_code err;
create_directories(FLAGS_minidump_path, err);
if (err) {
stringstream ss;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/network-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/network-util.cc b/be/src/util/network-util.cc
index afa5b32..0a722cc 100644
--- a/be/src/util/network-util.cc
+++ b/be/src/util/network-util.cc
@@ -23,6 +23,7 @@
#include <arpa/inet.h>
#include <limits.h>
#include <sstream>
+#include <random>
#include <vector>
#include <boost/algorithm/string.hpp>
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/parquet-reader.cc
----------------------------------------------------------------------
diff --git a/be/src/util/parquet-reader.cc b/be/src/util/parquet-reader.cc
index de11d7c..e6a9084 100644
--- a/be/src/util/parquet-reader.cc
+++ b/be/src/util/parquet-reader.cc
@@ -51,8 +51,7 @@ using namespace apache::thrift;
using namespace parquet;
using std::min;
-// Some code is replicated to make this more stand-alone.
-const uint8_t PARQUET_VERSION_NUMBER[] = {'P', 'A', 'R', '1'};
+using impala::PARQUET_VERSION_NUMBER;
boost::shared_ptr<TProtocol> CreateDeserializeProtocol(
boost::shared_ptr<TMemoryBuffer> mem, bool compact) {
@@ -221,6 +220,7 @@ int main(int argc, char** argv) {
uint8_t* buffer = reinterpret_cast<uint8_t*>(malloc(file_len));
size_t bytes_read = fread(buffer, 1, file_len, file);
assert(bytes_read == file_len);
+ (void)bytes_read;
// Check file starts and ends with magic bytes
assert(
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/periodic-counter-updater.h
----------------------------------------------------------------------
diff --git a/be/src/util/periodic-counter-updater.h b/be/src/util/periodic-counter-updater.h
index 161569c..893b887 100644
--- a/be/src/util/periodic-counter-updater.h
+++ b/be/src/util/periodic-counter-updater.h
@@ -102,7 +102,7 @@ class PeriodicCounterUpdater {
/// Loop for periodic counter update thread. This thread wakes up once in a while
/// and updates all the added rate counters and sampling counters.
- void UpdateLoop();
+ [[noreturn]] void UpdateLoop();
/// Thread performing asynchronous updates.
boost::scoped_ptr<boost::thread> update_thread_;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/pprof-path-handlers.cc
----------------------------------------------------------------------
diff --git a/be/src/util/pprof-path-handlers.cc b/be/src/util/pprof-path-handlers.cc
index 1c6f400..b8f245c 100644
--- a/be/src/util/pprof-path-handlers.cc
+++ b/be/src/util/pprof-path-handlers.cc
@@ -94,7 +94,7 @@ void PprofCpuProfileHandler(const Webserver::ArgumentMap& args, stringstream* ou
ProfilerStop();
ifstream prof_file(tmp_prof_file_name.str().c_str(), ios::in);
if (!prof_file.is_open()) {
- (*output) << "Unable to open cpu profile: " << tmp_prof_file_name;
+ (*output) << "Unable to open cpu profile: " << tmp_prof_file_name.str();
return;
}
(*output) << prof_file.rdbuf();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/progress-updater.cc
----------------------------------------------------------------------
diff --git a/be/src/util/progress-updater.cc b/be/src/util/progress-updater.cc
index ba0dfca..ad37510 100644
--- a/be/src/util/progress-updater.cc
+++ b/be/src/util/progress-updater.cc
@@ -52,7 +52,7 @@ void ProgressUpdater::Update(int64_t delta) {
if (num_complete >= total_) {
// Always print the final 100% complete
- VLOG(logging_level_) << label_ << " 100\% Complete ("
+ VLOG(logging_level_) << label_ << " 100% Complete ("
<< num_complete << " out of " << total_ << ")";
return;
}
@@ -62,7 +62,7 @@ void ProgressUpdater::Update(int64_t delta) {
if (new_percentage - old_percentage > update_period_) {
// Only update shared variable if this guy was the latest.
last_output_percentage_.CompareAndSwap(old_percentage, new_percentage);
- VLOG(logging_level_) << label_ << ": " << new_percentage << "\% Complete ("
+ VLOG(logging_level_) << label_ << ": " << new_percentage << "% Complete ("
<< num_complete << " out of " << total_ << ")";
}
}
@@ -72,11 +72,11 @@ string ProgressUpdater::ToString() const {
int64_t num_complete = num_complete_.Load();
if (num_complete >= total_) {
// Always print the final 100% complete
- ss << label_ << " 100\% Complete (" << num_complete << " out of " << total_ << ")";
+ ss << label_ << " 100% Complete (" << num_complete << " out of " << total_ << ")";
return ss.str();
}
int percentage = (static_cast<double>(num_complete) / total_) * 100;
- ss << label_ << ": " << percentage << "\% Complete ("
+ ss << label_ << ": " << percentage << "% Complete ("
<< num_complete << " out of " << total_ << ")";
return ss.str();
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/rle-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/rle-test.cc b/be/src/util/rle-test.cc
index 0e0368c..0501d71 100644
--- a/be/src/util/rle-test.cc
+++ b/be/src/util/rle-test.cc
@@ -102,9 +102,9 @@ TEST(BitArray, TestBool) {
// Writes 'num_vals' values with width 'bit_width' and reads them back.
void TestBitArrayValues(int bit_width, int num_vals) {
const int len = BitUtil::Ceil(bit_width * num_vals, 8);
- const uint64_t mod = bit_width == 64? 1 : 1LL << bit_width;
+ const uint64_t mod = bit_width == 64 ? 1 : 1LL << bit_width;
- uint8_t buffer[len];
+ uint8_t buffer[(len > 0) ? len : 1];
BitWriter writer(buffer, len);
for (int i = 0; i < num_vals; ++i) {
bool result = writer.PutValue(i % mod, bit_width);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/runtime-profile.h
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile.h b/be/src/util/runtime-profile.h
index c409d62..a514f8b 100644
--- a/be/src/util/runtime-profile.h
+++ b/be/src/util/runtime-profile.h
@@ -41,7 +41,8 @@ class ObjectPool;
/// corresponding rate based counter. This thread wakes up at fixed intervals and updates
/// all of the rate counters.
/// Thread-safe.
-class RuntimeProfile {
+class RuntimeProfile { // NOLINT: This struct is not packed, but there are not so many
+ // of them that it makes a performance difference
public:
class Counter {
public:
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/thread-pool.h
----------------------------------------------------------------------
diff --git a/be/src/util/thread-pool.h b/be/src/util/thread-pool.h
index d7be6dd..c5da7bd 100644
--- a/be/src/util/thread-pool.h
+++ b/be/src/util/thread-pool.h
@@ -23,6 +23,7 @@
#include <boost/thread/mutex.hpp>
#include <boost/bind/mem_fn.hpp>
+#include "util/aligned-new.h"
#include "util/thread.h"
namespace impala {
@@ -30,7 +31,7 @@ namespace impala {
/// Simple threadpool which processes items (of type T) in parallel which were placed on a
/// blocking queue by Offer(). Each item is processed by a single user-supplied method.
template <typename T>
-class ThreadPool {
+class ThreadPool : public CacheLineAligned {
public:
/// Signature of a work-processing function. Takes the integer id of the thread which is
/// calling it (ids run from 0 to num_threads - 1) and a reference to the item to
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/webserver.cc
----------------------------------------------------------------------
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index 41974bb..7cdd026 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -420,7 +420,13 @@ int Webserver::BeginRequestCallback(struct sq_connection* connection,
const string& str = output.str();
const string& headers = BuildHeaderString(response, content_type);
+
+ // printf with a non-literal format string is a security concern, but BuildHeaderString
+ // returns a limited set of strings and all members of that set are safe.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
sq_printf(connection, headers.c_str(), (int)str.length());
+#pragma clang diagnostic pop
// Make sure to use sq_write for printing the body; sq_printf truncates at 8kb
sq_write(connection, str.c_str(), str.length());
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/bin/make_impala.sh
----------------------------------------------------------------------
diff --git a/bin/make_impala.sh b/bin/make_impala.sh
index eb3ecf5..c198378 100755
--- a/bin/make_impala.sh
+++ b/bin/make_impala.sh
@@ -115,8 +115,10 @@ then
if [[ ! -z $IMPALA_TOOLCHAIN ]]; then
- if [[ "$TARGET_BUILD_TYPE" == "ADDRESS_SANITIZER" ]]; then
- CMAKE_ARGS+=(-DCMAKE_TOOLCHAIN_FILE=$IMPALA_HOME/cmake_modules/asan_toolchain.cmake)
+ if [[ ("$TARGET_BUILD_TYPE" == "ADDRESS_SANITIZER") \
+ || ("$TARGET_BUILD_TYPE" == "TIDY") ]]
+ then
+ CMAKE_ARGS+=(-DCMAKE_TOOLCHAIN_FILE=$IMPALA_HOME/cmake_modules/clang_toolchain.cmake)
else
CMAKE_ARGS+=(-DCMAKE_TOOLCHAIN_FILE=$IMPALA_HOME/cmake_modules/toolchain.cmake)
fi
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/bin/run_clang_tidy.sh
----------------------------------------------------------------------
diff --git a/bin/run_clang_tidy.sh b/bin/run_clang_tidy.sh
new file mode 100755
index 0000000..aa95845
--- /dev/null
+++ b/bin/run_clang_tidy.sh
@@ -0,0 +1,54 @@
+#!/bin/bash
+
+# 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.
+
+# This script runs clang-tidy on the backend, excluding gutil. clang-tidy finds bugs and
+# likely bugs.
+#
+# To use this script, the toolchain must be installed and the toolchain environment
+# variables must be set.
+#
+# The output of this script is quite verbose becuase clang-tidy and the clang-provided
+# run-clang-tidy.py have verbose output. The lines indicating likely bugs are the ones
+# that end in ']'.
+
+set -euo pipefail
+
+echo "Compiling"
+if ! ./buildall.sh -notests -tidy -so -noclean 1>/dev/null 2>/dev/null
+then
+ echo "WARNING: compile failed" >&2
+fi
+
+DIRS=$(ls -d "${IMPALA_HOME}/be/src/"*/ | grep -v gutil | tr '\n' ' ')
+PIPE_DIRS=$(echo "${DIRS}" | tr ' ' '|')
+
+# Reduce the concurrency to one less than the number of cores in the system. Note than
+# nproc may not be available on older distributions like Centos 5.5.
+if type nproc >/dev/null 2>&1; then
+ CORES=$(($(nproc) - 1))
+else
+ # This script has been tested when CORES is actually higher than the number of cores on
+ # the system, and the system remained responsive, so it's OK to guess here.
+ CORES=7
+fi
+
+export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/share/clang\
+:${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin/\
+:$PATH"
+run-clang-tidy.py -header-filter "${PIPE_DIRS%?}" -j"${CORES}" ${DIRS}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/buildall.sh
----------------------------------------------------------------------
diff --git a/buildall.sh b/buildall.sh
index d830ae3..a4791b7 100755
--- a/buildall.sh
+++ b/buildall.sh
@@ -58,6 +58,7 @@ MAKE_IMPALA_ARGS=""
CODE_COVERAGE=0
BUILD_ASAN=0
BUILD_FE_ONLY=0
+BUILD_TIDY=0
MAKE_CMD=make
LZO_CMAKE_ARGS=
@@ -109,6 +110,9 @@ do
-asan)
BUILD_ASAN=1
;;
+ -tidy)
+ BUILD_TIDY=1
+ ;;
-testpairwise)
EXPLORATION_STRATEGY=pairwise
;;
@@ -174,6 +178,7 @@ do
echo "[-release] : Release build [Default: debug]"
echo "[-codecoverage] : Build with code coverage [Default: False]"
echo "[-asan] : Address sanitizer build [Default: False]"
+ echo "[-tidy] : clang-tidy build [Default: False]"
echo "[-skiptests] : Skips execution of all tests"
echo "[-notests] : Skips building and execution of all tests"
echo "[-start_minicluster] : Start test cluster including Impala and all"\
@@ -248,6 +253,10 @@ if [[ ${BUILD_ASAN} -eq 1 ]]; then
fi
CMAKE_BUILD_TYPE=ADDRESS_SANITIZER
fi
+if [[ ${BUILD_TIDY} -eq 1 ]]; then
+ CMAKE_BUILD_TYPE=TIDY
+fi
+
MAKE_IMPALA_ARGS+=" -build_type=${CMAKE_BUILD_TYPE}"
# If we aren't kerberized then we certainly don't need to talk about
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/cmake_modules/asan_toolchain.cmake
----------------------------------------------------------------------
diff --git a/cmake_modules/asan_toolchain.cmake b/cmake_modules/asan_toolchain.cmake
deleted file mode 100644
index 1500e87..0000000
--- a/cmake_modules/asan_toolchain.cmake
+++ /dev/null
@@ -1,44 +0,0 @@
-# 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.
-
-# Set the root directory for the toolchain
-set(TOOLCHAIN_ROOT $ENV{IMPALA_TOOLCHAIN})
-
-# If Impala is built with the toolchain, change compiler and link paths
-set(GCC_ROOT $ENV{IMPALA_TOOLCHAIN}/gcc-$ENV{IMPALA_GCC_VERSION})
-
-# Use the appropriate LLVM version to build ASAN.
-set(LLVM_ASAN_ROOT $ENV{IMPALA_TOOLCHAIN}/llvm-$ENV{IMPALA_LLVM_ASAN_VERSION})
-
-set(CMAKE_C_COMPILER ${LLVM_ASAN_ROOT}/bin/clang)
-
-# Use clang to build unless overridden by environment.
-if($ENV{IMPALA_CXX_COMPILER} STREQUAL "default")
- set(CMAKE_CXX_COMPILER ${LLVM_ASAN_ROOT}/bin/clang++)
-else()
- set(CMAKE_CXX_COMPILER $ENV{IMPALA_CXX_COMPILER})
-endif()
-
-# Add the GCC root location to the compiler flags
-set(CXX_COMMON_FLAGS "--gcc-toolchain=${GCC_ROOT}")
-
-# The rpath is needed to be able to run the binaries produced by the toolchain without
-# specifying an LD_LIBRARY_PATH
-set(TOOLCHAIN_LINK_FLAGS "-Wl,-rpath,${GCC_ROOT}/lib64")
-set(TOOLCHAIN_LINK_FLAGS "${TOOLCHAIN_LINK_FLAGS} -L${GCC_ROOT}/lib64")
-
-message(STATUS "Setup toolchain link flags ${TOOLCHAIN_LINK_FLAGS}")
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/cmake_modules/clang_toolchain.cmake
----------------------------------------------------------------------
diff --git a/cmake_modules/clang_toolchain.cmake b/cmake_modules/clang_toolchain.cmake
new file mode 100644
index 0000000..9a44c1d
--- /dev/null
+++ b/cmake_modules/clang_toolchain.cmake
@@ -0,0 +1,54 @@
+# 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.
+
+# Set the root directory for the toolchain
+set(TOOLCHAIN_ROOT $ENV{IMPALA_TOOLCHAIN})
+
+# If Impala is built with the toolchain, change compiler and link paths
+set(GCC_ROOT $ENV{IMPALA_TOOLCHAIN}/gcc-$ENV{IMPALA_GCC_VERSION})
+
+# Use the appropriate LLVM version to build ASAN.
+set(LLVM_ASAN_ROOT $ENV{IMPALA_TOOLCHAIN}/llvm-$ENV{IMPALA_LLVM_ASAN_VERSION})
+
+set(LLVM_ROOT $ENV{IMPALA_TOOLCHAIN}/llvm-$ENV{IMPALA_LLVM_VERSION})
+
+if ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER")
+ set(CMAKE_C_COMPILER ${LLVM_ASAN_ROOT}/bin/clang)
+else()
+ set(CMAKE_C_COMPILER ${LLVM_ROOT}/bin/clang)
+endif()
+
+# Use clang to build unless overridden by environment.
+if($ENV{IMPALA_CXX_COMPILER} STREQUAL "default")
+ if ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER")
+ set(CMAKE_CXX_COMPILER ${LLVM_ASAN_ROOT}/bin/clang++)
+ else()
+ set(CMAKE_CXX_COMPILER ${LLVM_ROOT}/bin/clang++)
+ endif()
+else()
+ set(CMAKE_CXX_COMPILER $ENV{IMPALA_CXX_COMPILER})
+endif()
+
+# Add the GCC root location to the compiler flags
+set(CXX_COMMON_FLAGS "--gcc-toolchain=${GCC_ROOT}")
+
+# The rpath is needed to be able to run the binaries produced by the toolchain without
+# specifying an LD_LIBRARY_PATH
+set(TOOLCHAIN_LINK_FLAGS "-Wl,-rpath,${GCC_ROOT}/lib64")
+set(TOOLCHAIN_LINK_FLAGS "${TOOLCHAIN_LINK_FLAGS} -L${GCC_ROOT}/lib64")
+
+message(STATUS "Setup toolchain link flags ${TOOLCHAIN_LINK_FLAGS}")
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java b/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
index 4fcecb2..93f859d 100644
--- a/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
+++ b/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
@@ -95,25 +95,25 @@ public class BuiltinsDb extends Db {
private static final Map<Type, String> SAMPLE_SERIALIZE_SYMBOL =
ImmutableMap.<Type, String>builder()
.put(Type.BOOLEAN,
- "24ReservoirSampleSerializeIN10impala_udf10BooleanValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf10BooleanValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.TINYINT,
- "24ReservoirSampleSerializeIN10impala_udf10TinyIntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf10TinyIntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.SMALLINT,
- "24ReservoirSampleSerializeIN10impala_udf11SmallIntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf11SmallIntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.INT,
- "24ReservoirSampleSerializeIN10impala_udf6IntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf6IntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.BIGINT,
- "24ReservoirSampleSerializeIN10impala_udf9BigIntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf9BigIntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.FLOAT,
- "24ReservoirSampleSerializeIN10impala_udf8FloatValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf8FloatValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.DOUBLE,
- "24ReservoirSampleSerializeIN10impala_udf9DoubleValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf9DoubleValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.STRING,
- "24ReservoirSampleSerializeIN10impala_udf9StringValEEEKS3_PNS2_15FunctionContextERS4_")
+ "24ReservoirSampleSerializeIN10impala_udf9StringValEEES3_PNS2_15FunctionContextERKS3_")
.put(Type.TIMESTAMP,
- "24ReservoirSampleSerializeIN10impala_udf12TimestampValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf12TimestampValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.put(Type.DECIMAL,
- "24ReservoirSampleSerializeIN10impala_udf10DecimalValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_")
+ "24ReservoirSampleSerializeIN10impala_udf10DecimalValEEENS2_9StringValEPNS2_15FunctionContextERKS4_")
.build();
private static final Map<Type, String> SAMPLE_MERGE_SYMBOL =
[5/5] incubator-impala git commit: IMPALA-4379: Fix and test Kudu
table type checking, follow up
Posted by jb...@apache.org.
IMPALA-4379: Fix and test Kudu table type checking, follow up
The first fix for IMPALA-4379 went in before all comments
were addressed. First commit: 9b507b6.
This addresses some follow-up comments about how to handling
ALTER TABLE setting the storage_handler table property,
which doesn't really make sense to ever allow.
Change-Id: I93d04a04483af598b392c28874363e3b0202e1f3
Reviewed-on: http://gerrit.cloudera.org:8080/4894
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
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/32294220
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/32294220
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/32294220
Branch: refs/heads/master
Commit: 32294220c4a0a7a159a70cf8ef313622b7190303
Parents: c5f49ec
Author: Matthew Jacobs <mj...@cloudera.com>
Authored: Mon Oct 31 16:19:22 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Fri Nov 4 06:54:18 2016 +0000
----------------------------------------------------------------------
.../impala/analysis/AlterTableSetTblProperties.java | 14 +++++++++++---
.../org/apache/impala/service/CatalogOpExecutor.java | 3 +--
.../apache/impala/service/KuduCatalogOpExecutor.java | 14 ++++----------
.../org/apache/impala/analysis/AnalyzeDDLTest.java | 8 ++++++++
.../queries/QueryTest/kudu_alter.test | 5 -----
5 files changed, 24 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/32294220/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
index 1ff744d..f2a81cd 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
@@ -22,13 +22,15 @@ import java.util.List;
import java.util.Map;
import org.apache.avro.SchemaParseException;
+import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants;
import org.apache.hadoop.hive.serde2.avro.AvroSerdeUtils;
-
-import org.apache.impala.catalog.HdfsFileFormat;
import org.apache.impala.catalog.HdfsTable;
import org.apache.impala.catalog.Table;
import org.apache.impala.common.AnalysisException;
-import org.apache.impala.thrift.*;
+import org.apache.impala.thrift.TAlterTableParams;
+import org.apache.impala.thrift.TAlterTableSetTblPropertiesParams;
+import org.apache.impala.thrift.TAlterTableType;
+import org.apache.impala.thrift.TTablePropertyType;
import org.apache.impala.util.AvroSchemaParser;
import org.apache.impala.util.AvroSchemaUtils;
import org.apache.impala.util.MetaStoreUtil;
@@ -78,6 +80,12 @@ public class AlterTableSetTblProperties extends AlterTableSetStmt {
MetaStoreUtil.checkShortPropertyMap("Property", tblProperties_);
+ if (tblProperties_.containsKey(hive_metastoreConstants.META_TABLE_STORAGE)) {
+ throw new AnalysisException(String.format("Changing the '%s' table property is " +
+ "not supported to protect against metadata corruption.",
+ hive_metastoreConstants.META_TABLE_STORAGE));
+ }
+
// Check avro schema when it is set in avro.schema.url or avro.schema.literal to
// avoid potential metadata corruption (see IMPALA-2042).
// If both properties are set then only check avro.schema.literal and ignore
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/32294220/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index f5ffb59..ce5a1b2 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -2082,9 +2082,8 @@ public class CatalogOpExecutor {
tbl.getMetaStoreTable().deepCopy();
switch (params.getTarget()) {
case TBL_PROPERTY:
- boolean isKuduTable = KuduTable.isKuduTable(msTbl);
msTbl.getParameters().putAll(properties);
- if (isKuduTable) {
+ if (KuduTable.isKuduTable(msTbl)) {
KuduCatalogOpExecutor.validateKuduTblExists(msTbl);
}
break;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/32294220/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
index ad4f2a9..7f51717 100644
--- a/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
@@ -221,21 +221,15 @@ public class KuduCatalogOpExecutor {
}
/**
- * Validates the table properties of a Kudu table. It checks that the msTbl represents
- * a Kudu table (indicated by the Kudu storage handler), that the master
- * addresses point to valid Kudu masters, and that the table exists.
+ * Validates the table properties of a Kudu table. It checks that the master
+ * addresses point to valid Kudu masters and that the table exists.
* Throws an ImpalaRuntimeException if this is not the case.
*/
public static void validateKuduTblExists(
org.apache.hadoop.hive.metastore.api.Table msTbl) throws ImpalaRuntimeException {
- Map<String, String> properties = msTbl.getParameters();
- if (!KuduTable.isKuduTable(msTbl)) {
- throw new ImpalaRuntimeException(String.format("Table '%s' does not represent a " +
- "Kudu table. Expected storage_handler '%s' but found '%s'",
- msTbl.getTableName(), KuduTable.KUDU_STORAGE_HANDLER,
- properties.get(KuduTable.KEY_STORAGE_HANDLER)));
- }
+ Preconditions.checkArgument(KuduTable.isKuduTable(msTbl));
+ Map<String, String> properties = msTbl.getParameters();
String masterHosts = properties.get(KuduTable.KEY_MASTER_HOSTS);
Preconditions.checkState(!Strings.isNullOrEmpty(masterHosts));
String kuduTableName = properties.get(KuduTable.KEY_TABLE_NAME);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/32294220/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
index 730ca92..1f718d5 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
@@ -408,6 +408,11 @@ public class AnalyzeDDLTest extends FrontendTestBase {
+ "serdeproperties ('key'='" + long_property_value + "')",
"Property value length must be <= " + MetaStoreUtil.MAX_PROPERTY_VALUE_LENGTH
+ ": " + (MetaStoreUtil.MAX_PROPERTY_VALUE_LENGTH + 1));
+
+ AnalysisError(
+ "alter table functional.alltypes set tblproperties('storage_handler'='1')",
+ "Changing the 'storage_handler' table property is not supported to protect " +
+ "against metadata corruption.");
}
// Arbitrary exprs as partition key values. Constant exprs are ok.
@@ -1361,6 +1366,9 @@ public class AnalyzeDDLTest extends FrontendTestBase {
" stored as kudu as select vc from functional.chars_tiny",
"Cannot create table 't': Type VARCHAR(32) is not supported in Kudu");
AnalysisError("create table t primary key (id) distribute by hash into 3 buckets" +
+ " stored as kudu as select c1 as id from functional.decimal_tiny",
+ "Cannot create table 't': Type DECIMAL(10,4) is not supported in Kudu");
+ AnalysisError("create table t primary key (id) distribute by hash into 3 buckets" +
" stored as kudu as select id, s from functional.complextypes_fileformat",
"Expr 's' in select list returns a complex type 'STRUCT<f1:STRING,f2:INT>'.\n" +
"Only scalar types are allowed in the select list.");
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/32294220/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
index ad49c59..e6814e1 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
@@ -40,8 +40,3 @@ select count(*) from simple_new;
---- TYPES
BIGINT
====
----- QUERY
-alter table simple_new set tblproperties ('storage_handler'='foo')
----- CATCH
-ImpalaRuntimeException: Table 'simple_new' does not represent a Kudu table. Expected storage_handler 'com.cloudera.kudu.hive.KuduStorageHandler' but found 'foo'
-====
[3/5] incubator-impala git commit: IMPALA-3676: Use clang as a static
analysis tool
Posted by jb...@apache.org.
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 <jb...@cloudera.com>
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 <jb...@cloudera.com>
Authored: Fri Oct 7 19:33:28 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
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:
//
[4/5] incubator-impala git commit: IMPALA-4423: Correct but
conservative implementation of Subquery.equals().
Posted by jb...@apache.org.
IMPALA-4423: Correct but conservative implementation of Subquery.equals().
The underlying problem was for trivial/constant [NOT] EXISTS subqueries
we substituted out Subqueries with bool literals using an ExprSubstitutionMap,
but the Subquery.equals() function was not implemented properly, so we ended
up matching Subqueries to the wrong entry in the ExprSubstitutionMap.
This could ultimately lead to wrong plans and results.
Testing: Corrected an existing test and modified an existing test for
extra coverage.
Change-Id: I5562d98ce36507aa5e253323e184fd42b54f27ed
Reviewed-on: http://gerrit.cloudera.org:8080/4923
Reviewed-by: Alex Behm <al...@cloudera.com>
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/c5f49ec9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c5f49ec9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c5f49ec9
Branch: refs/heads/master
Commit: c5f49ec9bbc1b191545b6edd2848acb632b45973
Parents: 14891fe
Author: Alex Behm <al...@cloudera.com>
Authored: Wed Nov 2 10:54:32 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Fri Nov 4 00:19:33 2016 +0000
----------------------------------------------------------------------
.../org/apache/impala/analysis/Subquery.java | 17 +++++++++--
.../queries/PlannerTest/subquery-rewrite.test | 32 +++++++++++++++++---
2 files changed, 41 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c5f49ec9/fe/src/main/java/org/apache/impala/analysis/Subquery.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Subquery.java b/fe/src/main/java/org/apache/impala/analysis/Subquery.java
index f3dcb7c..414b98d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Subquery.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Subquery.java
@@ -20,14 +20,14 @@ package org.apache.impala.analysis;
import java.util.ArrayList;
import org.apache.hadoop.hive.metastore.MetaStoreUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import org.apache.impala.catalog.ArrayType;
import org.apache.impala.catalog.StructField;
import org.apache.impala.catalog.StructType;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.thrift.TExprNode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
@@ -149,6 +149,17 @@ public class Subquery extends Expr {
return new StructType(structFields);
}
+ /**
+ * Returns true if the toSql() of the Subqueries is identical. May return false for
+ * equivalent statements even due to minor syntactic differences like parenthesis.
+ * TODO: Switch to a less restrictive implementation.
+ */
+ @Override
+ public boolean equals(Object o) {
+ if (!super.equals(o)) return false;
+ return stmt_.toSql().equals(((Subquery)o).stmt_.toSql());
+ }
+
@Override
public Subquery clone() { return new Subquery(this); }
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c5f49ec9/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test b/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
index 1f52597..0880d9f 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
@@ -1732,8 +1732,8 @@ PLAN-ROOT SINK
partitions=11/11 files=11 size=814.73KB
runtime filters: RF001 -> tt1.int_col
====
-# Correlated EXISTS and NOT EXISTS subqueries with limit 0 and
-# aggregates. All predicates evaluate to FALSE. (IMPALA-1550)
+# IMPALA-1550/IMPALA-4423: Correlated EXISTS and NOT EXISTS subqueries with aggregates
+# that can be evaluated at query compile time. All predicates evaluate to FALSE.
select 1
from functional.alltypestiny t1
where exists
@@ -1744,13 +1744,17 @@ and not exists
(select count(distinct int_col)
from functional.alltypesagg t3
where t1.id = t3.id)
+and not exists
+ (select min(int_col)
+ from functional.alltypestiny t5
+ where t1.id = t5.id and false)
---- PLAN
PLAN-ROOT SINK
|
00:EMPTYSET
====
-# Correlated EXISTS and NOT EXISTS subqueries with limit 0 and
-# aggregates. All predicates evaluate to TRUE. (IMPALA-1550)
+# IMPALA-1550/IMPALA-4423: Correlated EXISTS and NOT EXISTS subqueries with aggregates
+# that can be evaluated at query compile time. All predicates evaluate to TRUE.
select 1
from functional.alltypestiny t1
where not exists
@@ -1768,7 +1772,7 @@ and not exists
and not exists
(select min(int_col)
from functional.alltypestiny t5
- where t1.id = t5.id and false)
+ where t1.id = t5.id having false)
---- PLAN
PLAN-ROOT SINK
|
@@ -2031,3 +2035,21 @@ PLAN-ROOT SINK
predicates: 20 >= t.bigint_col, t.string_col <= t.date_string_col
runtime filters: RF000 -> id
====
+# IMPALA-4423: Correlated EXISTS and NOT EXISTS subqueries with aggregates. Both
+# subqueries can be evaluated at query compile time. The first one evaluates to
+# TRUE and the second one to FALSE.
+select 1
+from functional.alltypestiny t1
+where not exists
+ (select id
+ from functional.alltypes t2
+ where t1.int_col = t2.int_col limit 0)
+and not exists
+ (select min(int_col)
+ from functional.alltypestiny t5
+ where t1.id = t5.id and false)
+---- PLAN
+PLAN-ROOT SINK
+|
+00:EMPTYSET
+====