You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2023/09/08 21:53:32 UTC
[impala] branch master updated (fcbd7f0d3 -> f563cce6b)
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from fcbd7f0d3 IMPALA-12273 Fix possible memory corruption of premature release of objects returned by native methods
new bc83d46a9 IMPALA-12424: Allow third party JniFrontend interface.
new f563cce6b IMPALA-12390 (part 1): Enable some clang-tidy performance related checks
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.clang-tidy | 6 +-
be/src/benchmarks/convert-timestamp-benchmark.cc | 2 +
be/src/catalog/catalog-server.cc | 2 +-
be/src/catalog/catalog-util.cc | 6 +-
be/src/codegen/llvm-codegen.cc | 1 +
be/src/common/logging.cc | 2 +-
be/src/exec/hdfs-columnar-scanner.cc | 1 +
be/src/exec/parquet/hdfs-parquet-scanner-test.cc | 1 +
be/src/exec/parquet/parquet-bool-decoder-test.cc | 1 +
be/src/exec/parquet/parquet-page-index-test.cc | 1 +
be/src/exprs/aggregate-functions-test.cc | 4 ++
be/src/exprs/anyval-util.cc | 1 +
be/src/exprs/timezone_db.cc | 2 +-
be/src/rpc/authentication.cc | 2 +-
be/src/rpc/rpc-mgr.cc | 6 +-
be/src/runtime/bufferpool/buffer-pool-test.cc | 3 +-
be/src/runtime/bufferpool/free-list-test.cc | 1 +
be/src/runtime/coordinator.cc | 4 +-
be/src/runtime/dml-exec-state.cc | 2 +-
be/src/runtime/io/data-cache-test.cc | 2 +-
be/src/runtime/io/disk-io-mgr-test.cc | 2 +
be/src/runtime/io/error-converter.cc | 2 +-
be/src/runtime/tmp-file-mgr-test.cc | 4 +-
be/src/runtime/tmp-file-mgr.cc | 2 +-
be/src/runtime/types.cc | 1 +
be/src/runtime/types.h | 4 +-
be/src/scheduling/admission-controller-test.cc | 2 +-
be/src/scheduling/admission-controller.cc | 5 +-
be/src/scheduling/cluster-membership-mgr.cc | 3 +-
be/src/service/client-request-state.cc | 2 +-
be/src/service/fe-support.cc | 1 +
be/src/service/frontend.cc | 8 ++-
be/src/service/impala-http-handler.cc | 6 +-
be/src/service/impala-server.cc | 6 +-
be/src/statestore/statestore-subscriber.cc | 1 +
be/src/udf/uda-test.cc | 1 +
be/src/udf_samples/uda-sample-test.cc | 2 +
be/src/util/dict-test.cc | 1 +
be/src/util/ldap-simple-bind.cc | 2 +-
be/src/util/mem-info.cc | 2 +-
be/src/util/rle-test.cc | 2 +
be/src/util/runtime-profile.cc | 2 +
be/src/util/simple-logger-test.cc | 4 +-
be/src/util/string-util.cc | 2 +-
be/src/util/webserver-test.cc | 4 +-
bin/start-impala-cluster.py | 7 ++
.../pom.xml | 15 ++--
.../apache/impala/external/TestJniFrontend.java | 80 ++++++++++++++++++++++
java/pom.xml | 1 +
..._configurations.py => test_external_planner.py} | 26 ++++---
50 files changed, 195 insertions(+), 55 deletions(-)
copy java/{test-corrupt-hive-udfs => external-frontend}/pom.xml (84%)
create mode 100644 java/external-frontend/src/main/java/org/apache/impala/external/TestJniFrontend.java
copy tests/custom_cluster/{test_disk_spill_configurations.py => test_external_planner.py} (57%)
[impala] 01/02: IMPALA-12424: Allow third party JniFrontend interface.
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit bc83d46a9a1b6b4a3ca859d3a2835185f4473427
Author: Steve Carlin <sc...@cloudera.com>
AuthorDate: Wed Sep 6 06:44:05 2023 -0700
IMPALA-12424: Allow third party JniFrontend interface.
This patch allows a third party to inject their own frontend
class instead of using the default JniFrontend included in the
project.
The test case includes an interface that runs queries as normal
except for the "select 1" query which gets changed to "select 42".
Change-Id: I89e677da557b39232847644b6ff17510e2b3c3d5
Reviewed-on: http://gerrit.cloudera.org:8080/20459
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/service/frontend.cc | 8 ++-
bin/start-impala-cluster.py | 7 ++
java/external-frontend/pom.xml | 56 +++++++++++++++
.../apache/impala/external/TestJniFrontend.java | 80 ++++++++++++++++++++++
java/pom.xml | 1 +
tests/custom_cluster/test_external_planner.py | 43 ++++++++++++
6 files changed, 194 insertions(+), 1 deletion(-)
diff --git a/be/src/service/frontend.cc b/be/src/service/frontend.cc
index 1d1fe51fc..4dcfd2506 100644
--- a/be/src/service/frontend.cc
+++ b/be/src/service/frontend.cc
@@ -84,6 +84,12 @@ DEFINE_string(kudu_master_hosts, "", "Specifies the default Kudu master(s). The
"optional.");
DEFINE_bool(enable_kudu_impala_hms_check, true, "By default this flag is true. If "
"enabled checks that Kudu and Impala are using the same HMS instance(s).");
+DEFINE_string(jni_frontend_class, "org/apache/impala/service/JniFrontend", "By default "
+ "the JniFrontend class included in the repository is used as the frontend interface. "
+ "This option allows the class to be overridden by a third party module. The "
+ "overridden class needs to contain all the methods in the methods[] variable, so "
+ "most implementations should make their class a child of JniFrontend and "
+ "override only relevant methods.");
Frontend::Frontend() {
JniMethodDescriptor methods[] = {
@@ -133,7 +139,7 @@ Frontend::Frontend() {
ABORT_IF_ERROR(jni_frame.push(jni_env));
// create instance of java class JniFrontend
- jclass fe_class = jni_env->FindClass("org/apache/impala/service/JniFrontend");
+ jclass fe_class = jni_env->FindClass(FLAGS_jni_frontend_class.c_str());
ABORT_IF_EXC(jni_env);
uint32_t num_methods = sizeof(methods) / sizeof(methods[0]);
diff --git a/bin/start-impala-cluster.py b/bin/start-impala-cluster.py
index 0f0c0e216..b2a5a48b4 100755
--- a/bin/start-impala-cluster.py
+++ b/bin/start-impala-cluster.py
@@ -155,6 +155,9 @@ parser.add_option("--enable_catalogd_ha", dest="enable_catalogd_ha",
action="store_true", default=False,
help="If true, enables CatalogD HA - the cluster will be launched "
"with two catalogd instances as Active-Passive HA pair.")
+parser.add_option("--jni_frontend_class", dest="jni_frontend_class",
+ action="store", default="org/apache/impala/service/JniFrontend",
+ help="Use a custom java frontend interface.")
# For testing: list of comma-separated delays, in milliseconds, that delay impalad catalog
# replica initialization. The ith delay is applied to the ith impalad.
@@ -485,6 +488,10 @@ def build_impalad_arg_lists(cluster_size, num_coordinators, use_exclusive_coordi
args = "{args} -geospatial_library={geospatial_library}".format(
args=args, geospatial_library=options.geospatial_library)
+ if options.jni_frontend_class:
+ args = "-jni_frontend_class={jni_frontend_class} {args}".format(
+ jni_frontend_class=options.jni_frontend_class, args=args)
+
# Appended at the end so they can override previous args.
if i < len(per_impalad_args):
args = "{args} {per_impalad_args}".format(
diff --git a/java/external-frontend/pom.xml b/java/external-frontend/pom.xml
new file mode 100644
index 000000000..c29fd78fd
--- /dev/null
+++ b/java/external-frontend/pom.xml
@@ -0,0 +1,56 @@
+<!--
+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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <groupId>org.apache.impala</groupId>
+ <artifactId>impala-parent</artifactId>
+ <version>4.3.0-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>external-frontend</artifactId>
+ <version>1.0</version>
+ <packaging>jar</packaging>
+
+ <name>external-frontend</name>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.impala</groupId>
+ <artifactId>impala-frontend</artifactId>
+ <version>4.3.0-SNAPSHOT</version>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.11.0</version>
+ <configuration>
+ <source>1.8</source>
+ <target>1.8</target>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+</project>
diff --git a/java/external-frontend/src/main/java/org/apache/impala/external/TestJniFrontend.java b/java/external-frontend/src/main/java/org/apache/impala/external/TestJniFrontend.java
new file mode 100644
index 000000000..0c4ef6b05
--- /dev/null
+++ b/java/external-frontend/src/main/java/org/apache/impala/external/TestJniFrontend.java
@@ -0,0 +1,80 @@
+/*
+ * 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.
+ */
+
+package org.apache.impala.externalfrontend;
+
+import com.google.common.base.Preconditions;
+import org.apache.impala.common.ImpalaException;
+import org.apache.impala.common.InternalException;
+import org.apache.impala.common.JniUtil;
+import org.apache.impala.authorization.AuthorizationFactory;
+import org.apache.impala.service.BackendConfig;
+import org.apache.impala.service.Frontend;
+import org.apache.impala.service.Frontend.PlanCtx;
+import org.apache.impala.service.JniFrontend;
+import org.apache.impala.thrift.TExecRequest;
+import org.apache.impala.thrift.TQueryCtx;
+import org.apache.impala.util.AuthorizationUtil;
+import org.apache.thrift.TException;
+
+import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.TSerializer;
+
+public class TestJniFrontend extends JniFrontend {
+ private final static TBinaryProtocol.Factory protocolFactory_ =
+ new TBinaryProtocol.Factory();
+
+ private final Frontend testFrontend_;
+
+ public TestJniFrontend(byte[] thriftBackendConfig, boolean isBackendTest)
+ throws ImpalaException, TException {
+ super(thriftBackendConfig, isBackendTest);
+ final AuthorizationFactory authzFactory =
+ AuthorizationUtil.authzFactoryFrom(BackendConfig.INSTANCE);
+ testFrontend_ = new Frontend(authzFactory, false);
+ }
+
+ /**
+ * Jni wrapper for Frontend.createExecRequest(). Accepts a serialized
+ * TQueryContext; returns a serialized TQueryExecRequest.
+ * This specific version will execute normally with the exception of the
+ * 'select 1' query which will instead implement a 'select 42' query, which
+ * is so bizarre, that it will prove that we hit this code. '42' also answers
+ * the question, "What is the meaning of life?"
+ */
+ @Override
+ public byte[] createExecRequest(byte[] thriftQueryContext)
+ throws ImpalaException {
+ Preconditions.checkNotNull(testFrontend_);
+ TQueryCtx queryCtx = new TQueryCtx();
+ JniUtil.deserializeThrift(protocolFactory_, queryCtx, thriftQueryContext);
+ if (queryCtx.getClient_request().getStmt().equals("select 1")) {
+ queryCtx.getClient_request().setStmt("select 42");
+ }
+
+ PlanCtx planCtx = new PlanCtx(queryCtx);
+ TExecRequest result = testFrontend_.createExecRequest(planCtx);
+
+ try {
+ TSerializer serializer = new TSerializer(protocolFactory_);
+ return serializer.serialize(result);
+ } catch (TException e) {
+ throw new InternalException(e.getMessage());
+ }
+ }
+}
diff --git a/java/pom.xml b/java/pom.xml
index 76364f06d..b5d6d0250 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -368,6 +368,7 @@ under the License.
<module>executor-deps</module>
<module>ext-data-source</module>
<module>../fe</module>
+ <module>external-frontend</module>
<module>query-event-hook-api</module>
<module>shaded-deps/hive-exec</module>
<module>shaded-deps/s3a-aws-sdk</module>
diff --git a/tests/custom_cluster/test_external_planner.py b/tests/custom_cluster/test_external_planner.py
new file mode 100644
index 000000000..17155ab74
--- /dev/null
+++ b/tests/custom_cluster/test_external_planner.py
@@ -0,0 +1,43 @@
+# 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.
+
+from __future__ import absolute_import, division, print_function
+import logging
+import pytest
+import os
+
+from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
+
+LOG = logging.getLogger(__name__)
+
+
+class TestExternalPlanner(CustomClusterTestSuite):
+
+ @classmethod
+ def setup_class(cls):
+ super(TestExternalPlanner, cls).setup_class()
+ os.environ["CUSTOM_CLASSPATH"] = os.getenv("IMPALA_HOME") + \
+ "/java/external-frontend/target/external-frontend-1.0.jar"
+
+ @pytest.mark.execute_serially
+ @CustomClusterTestSuite.with_args(
+ impalad_args="-jni_frontend_class=org/apache/impala/externalfrontend/TestJniFrontend")
+ def test_external_frontend(self, vector):
+ setup_client = self.create_impala_client()
+ assert setup_client.execute("select 2").data == ['2']
+ # The custom JniFrontend overrides the 'select 1' query with a 'select 42' query.
+ assert setup_client.execute("select 1").data == ['42']
[impala] 02/02: IMPALA-12390 (part 1): Enable some clang-tidy performance related checks
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit f563cce6b8eed4519772af06d89edc6e374839d3
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Mon Aug 21 15:42:00 2023 -0700
IMPALA-12390 (part 1): Enable some clang-tidy performance related checks
This enables several Clang Tidy performance checks and fixes
the flagged code locations. The specific checks enabled are:
1. performance-faster-string-find
"warning: 'find' called with a string literal consisting of a
single character; consider using the more effective overload
accepting a character"
Fix: Use char literals rather than string literals
2. performance-for-range-copy
"warning: loop variable is copied but only used as const reference;
consider making it a const reference"
Fix: Use const & for some locations of auto
3. performance-implicit-cast-in-loop
"warning: the type of the loop variable '$VAR' is different from
the one returned by the iterator and generates an implicit cast;
you can either change the type to the correct one ('$TYPE' but
'const auto&' is always an option) or remove the reference to make
it explicit that you are creating a new value"
Fix: Use the right type or const auto&
4. performance-inefficient-vector-operation
"warning: 'push_back' is called inside a loop; consider pre-allocating
the vector capacity before the loop"
Fix: Call reserve() on the vector before the loop
5. performance-type-promotion-in-math-fn - not encountered in our code
This disables a few performance checks temporarily to keep the
change a reasonable size.
Testing:
- Ran bin/run_clang_tidy.sh with the new checks
- Ran GVO
Change-Id: I3d5dfe04ffb4ec6f156e268c31a356651410ce91
Reviewed-on: http://gerrit.cloudera.org:8080/20387
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Michael Smith <mi...@cloudera.com>
---
.clang-tidy | 6 +++++-
be/src/benchmarks/convert-timestamp-benchmark.cc | 2 ++
be/src/catalog/catalog-server.cc | 2 +-
be/src/catalog/catalog-util.cc | 6 +++---
be/src/codegen/llvm-codegen.cc | 1 +
be/src/common/logging.cc | 2 +-
be/src/exec/hdfs-columnar-scanner.cc | 1 +
be/src/exec/parquet/hdfs-parquet-scanner-test.cc | 1 +
be/src/exec/parquet/parquet-bool-decoder-test.cc | 1 +
be/src/exec/parquet/parquet-page-index-test.cc | 1 +
be/src/exprs/aggregate-functions-test.cc | 4 ++++
be/src/exprs/anyval-util.cc | 1 +
be/src/exprs/timezone_db.cc | 2 +-
be/src/rpc/authentication.cc | 2 +-
be/src/rpc/rpc-mgr.cc | 6 +++---
be/src/runtime/bufferpool/buffer-pool-test.cc | 3 ++-
be/src/runtime/bufferpool/free-list-test.cc | 1 +
be/src/runtime/coordinator.cc | 4 +++-
be/src/runtime/dml-exec-state.cc | 2 +-
be/src/runtime/io/data-cache-test.cc | 2 +-
be/src/runtime/io/disk-io-mgr-test.cc | 2 ++
be/src/runtime/io/error-converter.cc | 2 +-
be/src/runtime/tmp-file-mgr-test.cc | 4 ++--
be/src/runtime/tmp-file-mgr.cc | 2 +-
be/src/runtime/types.cc | 1 +
be/src/runtime/types.h | 4 ++--
be/src/scheduling/admission-controller-test.cc | 2 +-
be/src/scheduling/admission-controller.cc | 5 +++--
be/src/scheduling/cluster-membership-mgr.cc | 3 ++-
be/src/service/client-request-state.cc | 2 +-
be/src/service/fe-support.cc | 1 +
be/src/service/impala-http-handler.cc | 6 +++---
be/src/service/impala-server.cc | 6 +++---
be/src/statestore/statestore-subscriber.cc | 1 +
be/src/udf/uda-test.cc | 1 +
be/src/udf_samples/uda-sample-test.cc | 2 ++
be/src/util/dict-test.cc | 1 +
be/src/util/ldap-simple-bind.cc | 2 +-
be/src/util/mem-info.cc | 2 +-
be/src/util/rle-test.cc | 2 ++
be/src/util/runtime-profile.cc | 2 ++
be/src/util/simple-logger-test.cc | 4 ++--
be/src/util/string-util.cc | 2 +-
be/src/util/webserver-test.cc | 4 ++--
44 files changed, 74 insertions(+), 39 deletions(-)
diff --git a/.clang-tidy b/.clang-tidy
index 47bce4d00..fa075dec2 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -73,7 +73,11 @@ Checks: "-*,clang*,\
-clang-diagnostic-used-but-marked-unused,\
-clang-diagnostic-vla-extension,\
-clang-diagnostic-weak-template-vtables,\
--clang-diagnostic-weak-vtables"
+-clang-diagnostic-weak-vtables,\
+performance-*,\
+-performance-inefficient-string-concatenation,\
+-performance-unnecessary-copy-initialization,\
+-performance-unnecessary-value-param"
# Ignore warnings in gutil
diff --git a/be/src/benchmarks/convert-timestamp-benchmark.cc b/be/src/benchmarks/convert-timestamp-benchmark.cc
index 84ba26ceb..a39829ab7 100644
--- a/be/src/benchmarks/convert-timestamp-benchmark.cc
+++ b/be/src/benchmarks/convert-timestamp-benchmark.cc
@@ -177,6 +177,7 @@ vector<TimestampValue> AddTestDataDateTimes(int n, const string& startstr) {
boost::posix_time::ptime start(boost::posix_time::time_from_string(startstr));
vector<TimestampValue> data;
+ data.reserve(n);
for (int i = 0; i < n; ++i) {
start += boost::gregorian::date_duration(dis_days(gen));
start += boost::posix_time::nanoseconds(dis_nanosec(gen));
@@ -216,6 +217,7 @@ public:
const vector<FROM>& data, const char* label) {
// Create TestData for each thread.
vector<unique_ptr<TestData<FROM, TO, converter>>> test_data;
+ test_data.reserve(num_of_threads);
for (int i = 0; i < num_of_threads; ++i) {
test_data.push_back(make_unique<TestData<FROM, TO, converter>>(data));
}
diff --git a/be/src/catalog/catalog-server.cc b/be/src/catalog/catalog-server.cc
index bcbd58e1e..30ae739dd 100644
--- a/be/src/catalog/catalog-server.cc
+++ b/be/src/catalog/catalog-server.cc
@@ -919,7 +919,7 @@ void CatalogServer::TableMetricsUrlCallback(const Webserver::WebRequest& req,
if (object_name_arg != args.end()) {
// Parse the object name to extract database and table names
const string& full_tbl_name = object_name_arg->second;
- int pos = full_tbl_name.find(".");
+ int pos = full_tbl_name.find('.');
if (pos == string::npos || pos >= full_tbl_name.size() - 1) {
stringstream error_msg;
error_msg << "Invalid table name: " << full_tbl_name;
diff --git a/be/src/catalog/catalog-util.cc b/be/src/catalog/catalog-util.cc
index 3b6616ba2..26c0b5dc4 100644
--- a/be/src/catalog/catalog-util.cc
+++ b/be/src/catalog/catalog-util.cc
@@ -165,7 +165,7 @@ Status TCatalogObjectFromObjectName(const TCatalogObjectType::type& object_type,
catalog_object->__set_type(object_type);
catalog_object->__set_table(TTable());
// Parse what should be a fully qualified table name
- int pos = object_name.find(".");
+ int pos = object_name.find('.');
if (pos == string::npos || pos >= object_name.size() - 1) {
stringstream error_msg;
error_msg << "Invalid table name: " << object_name;
@@ -181,8 +181,8 @@ Status TCatalogObjectFromObjectName(const TCatalogObjectType::type& object_type,
// db, fn and signature.
catalog_object->__set_type(object_type);
catalog_object->__set_fn(TFunction());
- int dot = object_name.find(".");
- int paren = object_name.find("(");
+ int dot = object_name.find('.');
+ int paren = object_name.find('(');
if (dot == string::npos || dot >= object_name.size() - 1 ||
paren == string::npos || paren >= object_name.size() - 1 ||
paren <= dot) {
diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index ac523da9d..b3ce78cd1 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -843,6 +843,7 @@ LlvmCodeGen::FnPrototype::FnPrototype(
llvm::Function* LlvmCodeGen::FnPrototype::GeneratePrototype(
LlvmBuilder* builder, llvm::Value** params) {
vector<llvm::Type*> arguments;
+ arguments.reserve(args_.size());
for (int i = 0; i < args_.size(); ++i) {
arguments.push_back(args_[i].type);
}
diff --git a/be/src/common/logging.cc b/be/src/common/logging.cc
index a7c04422c..825767ffa 100644
--- a/be/src/common/logging.cc
+++ b/be/src/common/logging.cc
@@ -61,7 +61,7 @@ string last_error_log_path = "";
void PrependFragment(string* s, bool* changed) {
impala::ThreadDebugInfo* tdi = impala::GetThreadDebugInfo();
if (tdi != nullptr) {
- for (auto id : { tdi->GetInstanceId(), tdi->GetQueryId() }) {
+ for (const auto& id : { tdi->GetInstanceId(), tdi->GetQueryId() }) {
if (id == ZERO_UNIQUE_ID) continue;
s->insert(0, PrintId(id) + "] ");
if (changed != nullptr) *changed = true;
diff --git a/be/src/exec/hdfs-columnar-scanner.cc b/be/src/exec/hdfs-columnar-scanner.cc
index fd741eecd..518091612 100644
--- a/be/src/exec/hdfs-columnar-scanner.cc
+++ b/be/src/exec/hdfs-columnar-scanner.cc
@@ -182,6 +182,7 @@ HdfsColumnarScanner::DivideReservationBetweenColumnsHelper(int64_t min_buffer_si
int64_t reservation_to_distribute) {
// Pair of (column index, reservation allocated).
ColumnReservations tmp_reservations;
+ tmp_reservations.reserve(col_range_lengths.size());
for (int i = 0; i < col_range_lengths.size(); ++i) tmp_reservations.emplace_back(i, 0);
// Sort in descending order of length, breaking ties by index so that larger columns
diff --git a/be/src/exec/parquet/hdfs-parquet-scanner-test.cc b/be/src/exec/parquet/hdfs-parquet-scanner-test.cc
index 5b55732c4..823c58d3c 100644
--- a/be/src/exec/parquet/hdfs-parquet-scanner-test.cc
+++ b/be/src/exec/parquet/hdfs-parquet-scanner-test.cc
@@ -100,6 +100,7 @@ TEST_F(HdfsParquetScannerTest, ComputeIdealReservation) {
// Test sum of reservations that doesn't fit in int32.
vector<int64_t> col_range_lengths;
const int64_t LARGE_NUM_RANGES = 10000;
+ col_range_lengths.reserve(LARGE_NUM_RANGES);
for (int i = 0; i < LARGE_NUM_RANGES; ++i) {
col_range_lengths.push_back(4 * MAX_BUFFER_SIZE);
}
diff --git a/be/src/exec/parquet/parquet-bool-decoder-test.cc b/be/src/exec/parquet/parquet-bool-decoder-test.cc
index ded6c242d..10dbdc18b 100644
--- a/be/src/exec/parquet/parquet-bool-decoder-test.cc
+++ b/be/src/exec/parquet/parquet-bool-decoder-test.cc
@@ -73,6 +73,7 @@ void TestSkipping(parquet::Encoding::type encoding, uint8_t* encoded_data,
TEST(ParquetBoolDecoder, TestDecodeAndSkipping) {
vector<bool> expected_data;
// Write 100 falses, 100 trues, 100 alternating falses and trues, 100 falses
+ expected_data.reserve(400);
for (int i = 0; i < 100; ++i) expected_data.push_back(false);
for (int i = 0; i < 100; ++i) expected_data.push_back(true);
for (int i = 0; i < 100; ++i) expected_data.push_back(i % 2);
diff --git a/be/src/exec/parquet/parquet-page-index-test.cc b/be/src/exec/parquet/parquet-page-index-test.cc
index f8a9a9c7b..6bf9d5f56 100644
--- a/be/src/exec/parquet/parquet-page-index-test.cc
+++ b/be/src/exec/parquet/parquet-page-index-test.cc
@@ -108,6 +108,7 @@ TEST(ParquetPageIndex, DeterminePageIndexRangesInRowGroup) {
template <typename T>
vector<string> ToStringVector(vector<T> vec) {
vector<string> result;
+ result.reserve(vec.size());
for (auto v : vec) {
result.emplace_back(string(reinterpret_cast<char*>(new T(v)), sizeof(v)));
}
diff --git a/be/src/exprs/aggregate-functions-test.cc b/be/src/exprs/aggregate-functions-test.cc
index b3f3b86f9..be061e489 100644
--- a/be/src/exprs/aggregate-functions-test.cc
+++ b/be/src/exprs/aggregate-functions-test.cc
@@ -98,6 +98,7 @@ TEST(HistogramTest, TestInt) {
// TODO: Add more deterministic test cases
{
vector<IntVal> input;
+ input.reserve(INPUT_SIZE);
for (int i = 0; i < INPUT_SIZE; ++i) input.push_back(i);
test_histogram.SetResultComparator(CheckHistogramDistribution);
StringVal max_expected_stdev = StringVal("100.0");
@@ -123,6 +124,7 @@ TEST(HistogramTest, TestDecimal) {
// All input values are x, result should be constant.
{
vector<DecimalVal> input;
+ input.reserve(INPUT_SIZE);
__int128_t val = MAX_UNSCALED_DECIMAL16;
stringstream ss;
for (int i = 0; i < INPUT_SIZE; ++i) input.push_back(DecimalVal(val));
@@ -135,6 +137,7 @@ TEST(HistogramTest, TestDecimal) {
{
vector<DecimalVal> input;
+ input.reserve(INPUT_SIZE);
for (int i = 0; i < INPUT_SIZE; ++i) input.push_back(DecimalVal(i));
test.SetResultComparator(CheckHistogramDistribution);
StringVal max_expected_stdev = StringVal("100.0");
@@ -154,6 +157,7 @@ TEST(HistogramTest, TestString) {
// All input values are x, result should be constant.
vector<StringVal> input;
+ input.reserve(INPUT_SIZE);
for (int i = 0; i < INPUT_SIZE; ++i) input.push_back(StringVal("x"));
char expected[] = "x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, "
"x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, "
diff --git a/be/src/exprs/anyval-util.cc b/be/src/exprs/anyval-util.cc
index fdff6583e..4cb9528b6 100644
--- a/be/src/exprs/anyval-util.cc
+++ b/be/src/exprs/anyval-util.cc
@@ -102,6 +102,7 @@ FunctionContext::TypeDesc AnyValUtil::ColumnTypeToTypeDesc(const ColumnType& typ
vector<FunctionContext::TypeDesc> AnyValUtil::ColumnTypesToTypeDescs(
const vector<ColumnType>& types) {
vector<FunctionContext::TypeDesc> type_descs;
+ type_descs.reserve(types.size());
for (const ColumnType& type : types) type_descs.push_back(ColumnTypeToTypeDesc(type));
return type_descs;
}
diff --git a/be/src/exprs/timezone_db.cc b/be/src/exprs/timezone_db.cc
index 56aa66a13..9ecedbe11 100644
--- a/be/src/exprs/timezone_db.cc
+++ b/be/src/exprs/timezone_db.cc
@@ -180,7 +180,7 @@ string TimezoneDatabase::LocalZoneName() {
string result;
while (timezone_file) {
getline(timezone_file, result);
- if (result.rfind("#", 0) == 0) continue;
+ if (result.rfind('#', 0) == 0) continue;
auto p = result.find("ZONE=\"");
if (p != string::npos) {
result.erase(p, p + 6);
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index 5dadd8e3a..cdc390916 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -503,7 +503,7 @@ int SaslAuthorizeInternal(sasl_conn_t* conn, void* context,
// Takes a Kerberos principal (either user/hostname@realm or user@realm)
// and returns the username part.
string GetShortUsernameFromKerberosPrincipal(const string& principal) {
- size_t end_idx = min(principal.find("/"), principal.find("@"));
+ size_t end_idx = min(principal.find('/'), principal.find('@'));
string short_user(
end_idx == string::npos || end_idx == 0 ?
principal : principal.substr(0, end_idx));
diff --git a/be/src/rpc/rpc-mgr.cc b/be/src/rpc/rpc-mgr.cc
index bcbf9d74a..727e84800 100644
--- a/be/src/rpc/rpc-mgr.cc
+++ b/be/src/rpc/rpc-mgr.cc
@@ -269,13 +269,13 @@ Status RpcMgr::StartServices() {
void RpcMgr::Join() {
if (services_started_) {
if (messenger_.get() == nullptr) return;
- for (auto service_pool : service_pools_) service_pool->Join();
+ for (const auto& service_pool : service_pools_) service_pool->Join();
}
}
void RpcMgr::Shutdown() {
if (messenger_.get() == nullptr) return;
- for (auto service_pool : service_pools_) service_pool->Shutdown();
+ for (const auto& service_pool : service_pools_) service_pool->Shutdown();
acceptor_pool_.reset();
messenger_->UnregisterAllServices();
@@ -377,7 +377,7 @@ void RpcMgr::ToJson(Document* document) {
// Add service pool metrics
Value services(kArrayType);
- for (auto service_pool : service_pools_) {
+ for (const auto& service_pool : service_pools_) {
Value service_entry(kObjectType);
service_pool->ToJson(&service_entry, document);
services.PushBack(service_entry, document->GetAllocator());
diff --git a/be/src/runtime/bufferpool/buffer-pool-test.cc b/be/src/runtime/bufferpool/buffer-pool-test.cc
index c322d026c..2348adef9 100644
--- a/be/src/runtime/bufferpool/buffer-pool-test.cc
+++ b/be/src/runtime/bufferpool/buffer-pool-test.cc
@@ -100,7 +100,7 @@ class BufferPoolTest : public ::testing::Test {
obj_pool_.Clear();
// Tests modify permissions, so make sure we can delete if they didn't clean up.
- for (string created_tmp_dir : created_tmp_dirs_) {
+ for (const string& created_tmp_dir : created_tmp_dirs_) {
chmod((created_tmp_dir + SCRATCH_SUFFIX).c_str(), S_IRWXU);
}
ASSERT_OK(FileSystemUtil::RemovePaths(created_tmp_dirs_));
@@ -408,6 +408,7 @@ class BufferPoolTest : public ::testing::Test {
// pages.
static string TmpFilePaths(vector<PageHandle>& pages) {
vector<string> paths;
+ paths.reserve(pages.size());
for (PageHandle& page : pages) {
paths.push_back(TmpFilePath(&page));
}
diff --git a/be/src/runtime/bufferpool/free-list-test.cc b/be/src/runtime/bufferpool/free-list-test.cc
index 5a025bf9f..86851a8b2 100644
--- a/be/src/runtime/bufferpool/free-list-test.cc
+++ b/be/src/runtime/bufferpool/free-list-test.cc
@@ -55,6 +55,7 @@ class FreeListTest : public ::testing::Test {
vector<const void*> GetSortedAddrs(const vector<BufferHandle>& buffers) {
vector<const void*> addrs;
+ addrs.reserve(buffers.size());
for (const BufferHandle& buffer : buffers) addrs.push_back(buffer.data());
std::sort(addrs.begin(), addrs.end());
return addrs;
diff --git a/be/src/runtime/coordinator.cc b/be/src/runtime/coordinator.cc
index 1bce78dde..90592ac2c 100644
--- a/be/src/runtime/coordinator.cc
+++ b/be/src/runtime/coordinator.cc
@@ -1167,7 +1167,7 @@ Status Coordinator::UpdateBlacklistWithAuxErrorInfo(
// the failed RPC. Only blacklist one node per ReportExecStatusRequestPB to avoid
// blacklisting nodes too aggressively. Currently, only blacklist the first node
// that contains a valid RPCErrorInfoPB object.
- for (auto aux_error : *aux_error_info) {
+ for (const auto& aux_error : *aux_error_info) {
if (aux_error.has_rpc_error_info()) {
RPCErrorInfoPB rpc_error_info = aux_error.rpc_error_info();
DCHECK(rpc_error_info.has_dest_node());
@@ -1257,6 +1257,7 @@ void Coordinator::HandleFailedExecRpcs(vector<BackendState*> failed_backend_stat
// Create an error based on the Exec RPC failure Status
vector<string> backend_addresses;
+ backend_addresses.reserve(failed_backend_states.size());
for (BackendState* backend_state : failed_backend_states) {
backend_addresses.push_back(
NetworkAddressPBToString(backend_state->krpc_impalad_address()));
@@ -1429,6 +1430,7 @@ void Coordinator::ReleaseBackendAdmissionControlResources(
parent_request_state_->admission_control_client();
DCHECK(admission_control_client != nullptr);
vector<NetworkAddressPB> host_addrs;
+ host_addrs.reserve(backend_states.size());
for (auto backend_state : backend_states) {
host_addrs.push_back(backend_state->impalad_address());
}
diff --git a/be/src/runtime/dml-exec-state.cc b/be/src/runtime/dml-exec-state.cc
index 61283d3ae..861ab4210 100644
--- a/be/src/runtime/dml-exec-state.cc
+++ b/be/src/runtime/dml-exec-state.cc
@@ -361,7 +361,7 @@ void DmlExecState::PopulatePathPermissionCache(hdfsFS fs, const string& path_str
string stripped_str;
if (scheme_end != string::npos) {
// Skip past the subsequent location:port/ prefix.
- stripped_str = path_str.substr(path_str.find("/", scheme_end + 3));
+ stripped_str = path_str.substr(path_str.find('/', scheme_end + 3));
} else {
stripped_str = path_str;
}
diff --git a/be/src/runtime/io/data-cache-test.cc b/be/src/runtime/io/data-cache-test.cc
index 8735bf518..2303d3146 100644
--- a/be/src/runtime/io/data-cache-test.cc
+++ b/be/src/runtime/io/data-cache-test.cc
@@ -733,7 +733,7 @@ TEST_P(DataCacheTest, AccessTraceAnonymization) {
ASSERT_OK(SimpleLogger::GetLogFiles(trace_dir, trace::TRACE_FILE_PREFIX,
&trace_files));
- for (string filename : trace_files) {
+ for (const string& filename : trace_files) {
trace::TraceFileIterator trace_file_iter(filename);
ASSERT_OK(trace_file_iter.Init());
while (true) {
diff --git a/be/src/runtime/io/disk-io-mgr-test.cc b/be/src/runtime/io/disk-io-mgr-test.cc
index 39aafed0e..645e0962f 100644
--- a/be/src/runtime/io/disk-io-mgr-test.cc
+++ b/be/src/runtime/io/disk-io-mgr-test.cc
@@ -1000,6 +1000,7 @@ TEST_F(DiskIoMgrTest, MemScarcity) {
unique_ptr<RequestContext> reader = io_mgr.RegisterContext();
vector<ScanRange*> ranges;
+ ranges.reserve(num_ranges);
for (int i = 0; i < num_ranges; ++i) {
ranges.push_back(InitRange(&pool_, tmp_file, 0, DATA_BYTES, 0, stat_val.st_mtime));
}
@@ -1482,6 +1483,7 @@ TEST_F(DiskIoMgrTest, SkipAllocateBuffers) {
// We should not read past the end of file.
vector<ScanRange*> ranges;
+ ranges.reserve(4);
for (int i = 0; i < 4; ++i) {
ranges.push_back(InitRange(&pool_, tmp_file, 0, len, 0, stat_val.st_mtime));
}
diff --git a/be/src/runtime/io/error-converter.cc b/be/src/runtime/io/error-converter.cc
index 6e2d0bfe0..2e8aff24c 100644
--- a/be/src/runtime/io/error-converter.cc
+++ b/be/src/runtime/io/error-converter.cc
@@ -100,7 +100,7 @@ bool ErrorConverter::IsBlacklistableError(const Status& status) {
size_t found = status.msg().msg().find("errno=");
if (found == string::npos) return false;
size_t start_pos = found + 6;
- size_t end_pos = status.msg().msg().find(",", start_pos);
+ size_t end_pos = status.msg().msg().find(',', start_pos);
string value = (end_pos != string::npos) ?
status.msg().msg().substr(start_pos, end_pos - start_pos) :
status.msg().msg().substr(start_pos);
diff --git a/be/src/runtime/tmp-file-mgr-test.cc b/be/src/runtime/tmp-file-mgr-test.cc
index 69b80c975..1ff4d300e 100644
--- a/be/src/runtime/tmp-file-mgr-test.cc
+++ b/be/src/runtime/tmp-file-mgr-test.cc
@@ -1094,7 +1094,7 @@ TEST_F(TmpFileMgrTest, TestDirectoryLimitParsingRemotePath) {
// Two types of paths, one with directory, one without.
vector<string> hdfs_paths{
test_env_->GetDefaultFsPath(""), test_env_->GetDefaultFsPath("/tmp")};
- for (string hdfs_path : hdfs_paths) {
+ for (const string& hdfs_path : hdfs_paths) {
string full_hdfs_path = hdfs_path + "/impala-scratch";
auto& dirs1 = GetTmpRemoteDir(CreateTmpFileMgr(hdfs_path + ",/tmp/local-buffer-dir"));
EXPECT_NE(nullptr, dirs1);
@@ -1166,7 +1166,7 @@ TEST_F(TmpFileMgrTest, TestDirectoryLimitParsingRemotePath) {
fake_hdfs_conn_map.insert(make_pair("s3a://fake_host/", fake_conn));
// Two types of paths, one with directory, one without.
vector<string> s3_paths{"s3a://fake_host", "s3a://fake_host/dir"};
- for (string s3_path : s3_paths) {
+ for (const string& s3_path : s3_paths) {
string full_s3_path = s3_path + "/impala-scratch";
auto& dirs13 = GetTmpRemoteDir(
CreateTmpFileMgr("/tmp/local-buffer-dir, " + s3_path, true, &fake_hdfs_conn_map));
diff --git a/be/src/runtime/tmp-file-mgr.cc b/be/src/runtime/tmp-file-mgr.cc
index ea871700c..c7039f776 100644
--- a/be/src/runtime/tmp-file-mgr.cc
+++ b/be/src/runtime/tmp-file-mgr.cc
@@ -817,7 +817,7 @@ Status TmpDirHdfs::ParsePathTokens(vector<string>& toks) {
split(toks, raw_path_, is_any_of(":"), token_compress_off);
// Only called on paths starting with `hdfs://` or `ofs://`.
DCHECK(toks.size() >= 2);
- if (toks[1].rfind("/") > 1) {
+ if (toks[1].rfind('/') > 1) {
// Contains a slash after the scheme, so port number was omitted.
toks[0] = Substitute("$0:$1", toks[0], toks[1]);
toks.erase(toks.begin()+1);
diff --git a/be/src/runtime/types.cc b/be/src/runtime/types.cc
index 5ae510ff3..cf7ba89b4 100644
--- a/be/src/runtime/types.cc
+++ b/be/src/runtime/types.cc
@@ -282,6 +282,7 @@ string ColumnType::DebugString() const {
vector<ColumnType> ColumnType::FromThrift(const vector<TColumnType>& ttypes) {
vector<ColumnType> types;
+ types.reserve(ttypes.size());
for (const TColumnType& ttype : ttypes) types.push_back(FromThrift(ttype));
return types;
}
diff --git a/be/src/runtime/types.h b/be/src/runtime/types.h
index 63a8600bb..d899e20f2 100644
--- a/be/src/runtime/types.h
+++ b/be/src/runtime/types.h
@@ -286,7 +286,7 @@ struct ColumnType {
switch (col_type.type) {
case TYPE_STRUCT: {
int struct_size = 0;
- for (ColumnType child_type : col_type.children) {
+ for (const ColumnType& child_type : col_type.children) {
struct_size += GetSlotSize(child_type);
}
return struct_size;
@@ -310,7 +310,7 @@ struct ColumnType {
switch (col_type.type) {
case TYPE_STRUCT: {
int struct_size = 0;
- for (ColumnType child_type : col_type.children) {
+ for (const ColumnType& child_type : col_type.children) {
struct_size += GetByteSize(child_type);
}
return struct_size;
diff --git a/be/src/scheduling/admission-controller-test.cc b/be/src/scheduling/admission-controller-test.cc
index f2d01ba15..9111a5367 100644
--- a/be/src/scheduling/admission-controller-test.cc
+++ b/be/src/scheduling/admission-controller-test.cc
@@ -186,7 +186,7 @@ class AdmissionControllerTest : public testing::Test {
/// Set the slots in use for all the hosts in 'host_addrs'.
void SetSlotsInUse(AdmissionController* admission_controller,
const vector<NetworkAddressPB>& host_addrs, int slots_in_use) {
- for (NetworkAddressPB host_addr : host_addrs) {
+ for (const NetworkAddressPB& host_addr : host_addrs) {
string host = NetworkAddressPBToString(host_addr);
admission_controller->host_stats_[host].slots_in_use = slots_in_use;
}
diff --git a/be/src/scheduling/admission-controller.cc b/be/src/scheduling/admission-controller.cc
index 76bdd774c..6d53d1f03 100644
--- a/be/src/scheduling/admission-controller.cc
+++ b/be/src/scheduling/admission-controller.cc
@@ -545,6 +545,7 @@ string AdmissionController::GetLogStringForTopNQueriesInPool(
// Now we are ready to report the stats.
// Prepare an index object that indicates the reporting for all elements.
std::vector<int> indices;
+ indices.reserve(items);
for (int i=0; i<items; i++) indices.emplace_back(i);
int indent = 0;
stringstream ss;
@@ -782,7 +783,7 @@ void AdmissionController::PoolStats::Dequeue(bool timed_out) {
void AdmissionController::UpdateStatsOnReleaseForBackends(const UniqueIdPB& query_id,
RunningQuery& running_query, const vector<NetworkAddressPB>& host_addrs) {
int64_t total_mem_to_release = 0;
- for (auto host_addr : host_addrs) {
+ for (const auto& host_addr : host_addrs) {
auto backend_allocation = running_query.per_backend_resources.find(host_addr);
if (backend_allocation == running_query.per_backend_resources.end()) {
// In the context of the admission control service, this may happen, eg. if a
@@ -1548,7 +1549,7 @@ void AdmissionController::ReleaseQueryBackendsLocked(const UniqueIdPB& query_id,
if (VLOG_IS_ON(2)) {
stringstream ss;
ss << "Released query backend(s) ";
- for (auto host_addr : host_addrs) ss << host_addr << " ";
+ for (const auto& host_addr : host_addrs) ss << host_addr << " ";
ss << "for query id=" << PrintId(query_id) << " "
<< GetPoolStats(running_query.request_pool)->DebugString();
VLOG(2) << ss.str();
diff --git a/be/src/scheduling/cluster-membership-mgr.cc b/be/src/scheduling/cluster-membership-mgr.cc
index 34e154273..426931a54 100644
--- a/be/src/scheduling/cluster-membership-mgr.cc
+++ b/be/src/scheduling/cluster-membership-mgr.cc
@@ -517,7 +517,7 @@ ClusterMembershipMgr::BeDescSharedPtr ClusterMembershipMgr::GetLocalBackendDescr
void ClusterMembershipMgr::NotifyListeners(SnapshotPtr snapshot) {
lock_guard<mutex> l(callback_fn_lock_);
- for (auto fn : update_callback_fns_) fn(snapshot);
+ for (const auto& fn : update_callback_fns_) fn(snapshot);
}
void ClusterMembershipMgr::SetState(const SnapshotPtr& new_state) {
@@ -695,6 +695,7 @@ void PopulateExecutorMembershipRequest(ClusterMembershipMgr::SnapshotPtr& snapsh
}
if (matching_exec_groups_found != snapshot->executor_groups.size()) {
vector<string> group_sets;
+ group_sets.reserve(exec_group_sets.size());
for (const auto& set : exec_group_sets) {
group_sets.push_back(set.exec_group_name_prefix);
}
diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc
index 0d48c921e..d20cbdee9 100644
--- a/be/src/service/client-request-state.cc
+++ b/be/src/service/client-request-state.cc
@@ -912,7 +912,7 @@ void ClientRequestState::ExecLoadIcebergDataRequestImpl(TLoadDataResp response)
lock_guard<mutex> l(lock_);
RETURN_VOID_IF_ERROR(UpdateQueryStatus(Status(revert_err + hdfs_ret.GetDetail())));
}
- for (string src_path : response.loaded_files) {
+ for (const string& src_path : response.loaded_files) {
hdfsFS hdfs_src_conn;
hdfs_ret = HdfsFsCache::instance()->GetConnection(dst_path, &hdfs_src_conn);
if (!hdfs_ret.ok()) {
diff --git a/be/src/service/fe-support.cc b/be/src/service/fe-support.cc
index 2d2fa7dc0..89e29c323 100644
--- a/be/src/service/fe-support.cc
+++ b/be/src/service/fe-support.cc
@@ -447,6 +447,7 @@ Java_org_apache_impala_service_FeSupport_NativeLookupSymbol(
JniUtil::internal_exc_class(), nullptr);
vector<ColumnType> arg_types;
+ arg_types.reserve(lookup.arg_types.size());
for (int i = 0; i < lookup.arg_types.size(); ++i) {
arg_types.push_back(ColumnType::FromThrift(lookup.arg_types[i]));
}
diff --git a/be/src/service/impala-http-handler.cc b/be/src/service/impala-http-handler.cc
index 5a4eca3d9..b17afb383 100644
--- a/be/src/service/impala-http-handler.cc
+++ b/be/src/service/impala-http-handler.cc
@@ -792,12 +792,12 @@ void ImpalaHttpHandler::FillClientHostsInfo(
total_connections += connection_ids.size();
Value hostname(pair.first.c_str(), document->GetAllocator());
client_host_json.AddMember("hostname", hostname, document->GetAllocator());
- for (TUniqueId connection_id : connection_ids) {
+ for (const TUniqueId& connection_id : connection_ids) {
ImpalaServer::ConnectionToSessionMap::iterator it =
server_->connection_to_sessions_map_.find(connection_id);
if (it != server_->connection_to_sessions_map_.end()) {
std::set<TUniqueId> session_ids = it->second;
- for (TUniqueId session_id : session_ids) {
+ for (const TUniqueId& session_id : session_ids) {
ImpalaServer::SessionStateMap::iterator session_state_map_iterator =
server_->session_state_map_.find(session_id);
if (session_state_map_iterator != server_->session_state_map_.end()
@@ -885,7 +885,7 @@ void ImpalaHttpHandler::FillConnectionsInfo(
server_->connection_to_sessions_map_.find(connection_context->connection_id);
if (it != server_->connection_to_sessions_map_.end()) {
// Filter out invalid session
- for (TUniqueId session_id : it->second) {
+ for (const TUniqueId& session_id : it->second) {
if (server_->session_state_map_.find(session_id)
!= server_->session_state_map_.end())
valid_session_ids.insert(session_id);
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 1a6863fd7..aaf4b6f12 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -634,7 +634,7 @@ Status ImpalaServer::PopulateAuthorizedProxyConfig(
token_compress_on);
if (proxy_config.size() > 0) {
for (const string& config: proxy_config) {
- size_t pos = config.find("=");
+ size_t pos = config.find('=');
if (pos == string::npos) {
return Status(config);
}
@@ -2060,7 +2060,7 @@ Status ImpalaServer::AuthorizeProxyUser(const string& user, const string& do_as_
// Get the short version of the user name (the user name up to the first '/' or '@')
// from the full principal name.
- size_t end_idx = min(user.find("/"), user.find("@"));
+ size_t end_idx = min(user.find('/'), user.find('@'));
// If neither are found (or are found at the beginning of the user name),
// return the username. Otherwise, return the username up to the matching character.
string short_user(
@@ -2117,7 +2117,7 @@ bool ImpalaServer::IsAuthorizedProxyUser(const string& user) {
// Get the short version of the user name (the user name up to the first '/' or '@')
// from the full principal name.
- size_t end_idx = min(user.find("/"), user.find("@"));
+ size_t end_idx = min(user.find('/'), user.find('@'));
// If neither are found (or are found at the beginning of the user name),
// return the username. Otherwise, return the username up to the matching character.
string short_user(
diff --git a/be/src/statestore/statestore-subscriber.cc b/be/src/statestore/statestore-subscriber.cc
index 2ef1ac5ee..b49e25491 100644
--- a/be/src/statestore/statestore-subscriber.cc
+++ b/be/src/statestore/statestore-subscriber.cc
@@ -696,6 +696,7 @@ Status StatestoreSubscriber::StatestoreStub::UpdateState(
// Put the updates into ascending order of topic name to match the lock acquisition
// order of TopicRegistration::update_lock.
vector<const TTopicDelta*> deltas_to_process;
+ deltas_to_process.reserve(incoming_topic_deltas.size());
for (auto& delta : incoming_topic_deltas) deltas_to_process.push_back(&delta.second);
sort(deltas_to_process.begin(), deltas_to_process.end(),
[](const TTopicDelta* left, const TTopicDelta* right) {
diff --git a/be/src/udf/uda-test.cc b/be/src/udf/uda-test.cc
index 485a7043d..137044517 100644
--- a/be/src/udf/uda-test.cc
+++ b/be/src/udf/uda-test.cc
@@ -300,6 +300,7 @@ TEST(MemTest, Basic) {
::MemTestInit, ::MemTestUpdate, ::MemTestMerge, ::MemTestSerialize,
::MemTestFinalize);
vector<BigIntVal> input;
+ input.reserve(10);
for (int i = 0; i < 10; ++i) {
input.push_back(10);
}
diff --git a/be/src/udf_samples/uda-sample-test.cc b/be/src/udf_samples/uda-sample-test.cc
index 6aacc87f8..766fe0201 100644
--- a/be/src/udf_samples/uda-sample-test.cc
+++ b/be/src/udf_samples/uda-sample-test.cc
@@ -60,6 +60,7 @@ bool TestAvg() {
test.SetIntermediateSize(16);
vector<DoubleVal> vals;
+ vals.reserve(1001);
for (int i = 0; i < 1001; ++i) {
vals.push_back(DoubleVal(i));
}
@@ -81,6 +82,7 @@ bool TestStringConcat() {
values.push_back("World");
vector<StringVal> separators;
+ separators.reserve(values.size());
for(int i = 0; i < values.size(); ++i) {
separators.push_back(",");
}
diff --git a/be/src/util/dict-test.cc b/be/src/util/dict-test.cc
index 097370740..d2cc61dc1 100644
--- a/be/src/util/dict-test.cc
+++ b/be/src/util/dict-test.cc
@@ -429,6 +429,7 @@ TEST(DictTest, TestSkippingValues) {
};
vector<int32_t> literal_values;
+ literal_values.reserve(200);
for (int i = 0; i < 200; ++i) literal_values.push_back(i);
ValidateSkipping(literal_values, literal_values, 0, 4);
ValidateSkipping(literal_values, literal_values, 0, 130);
diff --git a/be/src/util/ldap-simple-bind.cc b/be/src/util/ldap-simple-bind.cc
index 97ca6bafd..320995719 100644
--- a/be/src/util/ldap-simple-bind.cc
+++ b/be/src/util/ldap-simple-bind.cc
@@ -182,7 +182,7 @@ string LdapSimpleBind::ConstructUserDN(const string& user) {
string user_dn = user;
if (!FLAGS_ldap_domain.empty()) {
// Append @domain if there isn't already an @ in the user string.
- if (user_dn.find("@") == string::npos) {
+ if (user_dn.find('@') == string::npos) {
user_dn = Substitute("$0@$1", user_dn, FLAGS_ldap_domain);
}
} else if (!FLAGS_ldap_baseDN.empty()) {
diff --git a/be/src/util/mem-info.cc b/be/src/util/mem-info.cc
index 43cfd24ff..51c5ce3f1 100644
--- a/be/src/util/mem-info.cc
+++ b/be/src/util/mem-info.cc
@@ -130,7 +130,7 @@ MappedMemInfo MemInfo::ParseSmaps() {
size_t colon_pos = line.find(':');
if (colon_pos == string::npos) continue;
string name = line.substr(0, colon_pos);
- size_t non_space_after_colon_pos = line.find_first_not_of(" ", colon_pos + 1);
+ size_t non_space_after_colon_pos = line.find_first_not_of(' ', colon_pos + 1);
if (non_space_after_colon_pos == string::npos) continue;
// From the first non-space after the colon through the end of the string.
string value = line.substr(non_space_after_colon_pos);
diff --git a/be/src/util/rle-test.cc b/be/src/util/rle-test.cc
index d4534888b..a8a0812fb 100644
--- a/be/src/util/rle-test.cc
+++ b/be/src/util/rle-test.cc
@@ -230,6 +230,7 @@ class RleTest : public ::testing::Test {
void TestRleValues(int bit_width, int num_vals, int value = -1) {
const int64_t mod = (bit_width == 64) ? 1 : 1LL << bit_width;
vector<int> values;
+ values.reserve(num_vals);
for (int v = 0; v < num_vals; ++v) {
values.push_back((value != -1) ? value : (v % mod));
}
@@ -450,6 +451,7 @@ TEST_F(RleTest, BitWidthZeroLiteral) {
// group but flush before finishing.
TEST_F(RleTest, Flush) {
vector<int> values;
+ values.reserve(18);
for (int i = 0; i < 16; ++i) values.push_back(1);
values.push_back(0);
ValidateRle(values, 1, NULL, -1);
diff --git a/be/src/util/runtime-profile.cc b/be/src/util/runtime-profile.cc
index 75068bc68..b63cd5c99 100644
--- a/be/src/util/runtime-profile.cc
+++ b/be/src/util/runtime-profile.cc
@@ -1080,6 +1080,7 @@ void RuntimeProfile::SortChildrenByTotalTime() {
// Create a snapshot of total time values so that they don't change while we're
// sorting. Sort the <total_time, index> pairs, then reshuffle children_.
vector<pair<int64_t, int64_t>> total_times;
+ total_times.reserve(children_.size());
for (int i = 0; i < children_.size(); ++i) {
total_times.emplace_back(children_[i].first->total_time_counter()->value(), i);
}
@@ -1089,6 +1090,7 @@ void RuntimeProfile::SortChildrenByTotalTime() {
return p1.first > p2.first;
});
ChildVector new_children;
+ new_children.reserve(total_times.size());
for (const auto& p : total_times) new_children.emplace_back(children_[p.second]);
children_ = move(new_children);
}
diff --git a/be/src/util/simple-logger-test.cc b/be/src/util/simple-logger-test.cc
index cb47bf7aa..87921c65a 100644
--- a/be/src/util/simple-logger-test.cc
+++ b/be/src/util/simple-logger-test.cc
@@ -249,7 +249,7 @@ TEST_F(SimpleLoggerTest, Blast) {
&log_files);
ASSERT_OK(status);
// Debugging logging to help diagnosis for any problems.
- for (string log_file : log_files) {
+ for (const string& log_file : log_files) {
LOG(INFO) << "Log files after blast: " << log_file;
}
EXPECT_EQ(log_files.size(), MAX_LOG_FILES);
@@ -264,7 +264,7 @@ TEST_F(SimpleLoggerTest, Blast) {
// Regex to match the expect line. The parentheses are a grouping that lets us extract
// the number separately.
std::regex entry_regex = std::regex("entry_([0-9]+)");
- for (string log_file : log_files) {
+ for (const string& log_file : log_files) {
ifstream file(log_file);
string line;
while (getline(file, line)) {
diff --git a/be/src/util/string-util.cc b/be/src/util/string-util.cc
index 1c11acd60..676f01ee6 100644
--- a/be/src/util/string-util.cc
+++ b/be/src/util/string-util.cc
@@ -58,7 +58,7 @@ Status TruncateUp(const string& str, int32_t max_length, string* result) {
bool CommaSeparatedContains(const std::string& cs_list, const std::string& item) {
size_t pos = 0;
while (pos < cs_list.size()) {
- size_t comma_pos = cs_list.find(",", pos);
+ size_t comma_pos = cs_list.find(',', pos);
if (comma_pos == string::npos) return cs_list.compare(pos, string::npos, item) == 0;
if (cs_list.compare(pos, comma_pos - pos, item) == 0) return true;
pos = comma_pos + 1;
diff --git a/be/src/util/webserver-test.cc b/be/src/util/webserver-test.cc
index 2ed6d567f..824ff6ce5 100644
--- a/be/src/util/webserver-test.cc
+++ b/be/src/util/webserver-test.cc
@@ -89,7 +89,7 @@ struct HttpRequest {
if (method == "POST") {
request_stream << "Content-Length: 0\r\n";
}
- for (const std::pair<string, string>& header : headers) {
+ for (const auto& header : headers) {
request_stream << header.first << ": " << header.second << "\r\n";
}
@@ -449,7 +449,7 @@ TEST(Webserver, SslGoodTlsVersion) {
ASSERT_OK(webserver.Start());
}
- for (auto v : unsupported_versions) {
+ for (const auto& v : unsupported_versions) {
auto ssl_version = ScopedFlagSetter<string>::Make(&FLAGS_ssl_minimum_version, v);
MetricGroup metrics("webserver-test");