You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/09/29 00:32:37 UTC

[5/6] incubator-impala git commit: IMPALA-3308: Get expr-test passing on PPC64LE

IMPALA-3308: Get expr-test passing on PPC64LE

When using gcc 5+ (which introduced a new library ABI that includes new
implementations of std::string) to build Impala, the copy of the class
ExprValue(std::string) would be unsafe as string_val.ptr will not be
updated to point to the relocated string_data.data().

In order to solve this issue, we need to change how we initialize value_
so that it is initialized in-place, rather than created as a temporary
on the stack and then copied.

Change-Id: I4504ee6a52a085f530aadfcfa009bacb83c64787
Reviewed-on: http://gerrit.cloudera.org:8080/4186
Reviewed-by: Tim Armstrong <ta...@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/9cee2b5f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/9cee2b5f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/9cee2b5f

Branch: refs/heads/master
Commit: 9cee2b5f1376ad6286ed65edffe6d152a0012cf1
Parents: 1ccd83b
Author: segelyang <zh...@cn.ibm.com>
Authored: Wed Aug 31 18:13:17 2016 +0800
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Wed Sep 28 23:09:28 2016 +0000

----------------------------------------------------------------------
 be/src/exprs/expr-value.h |  9 +++++----
 be/src/exprs/literal.cc   | 14 ++++++--------
 2 files changed, 11 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/9cee2b5f/be/src/exprs/expr-value.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-value.h b/be/src/exprs/expr-value.h
index cbe1361..a2336d3 100644
--- a/be/src/exprs/expr-value.h
+++ b/be/src/exprs/expr-value.h
@@ -67,10 +67,9 @@ struct ExprValue {
   ExprValue(double v) : double_val(v) {}
   ExprValue(int64_t t, int64_t n) : timestamp_val(t, n) {}
 
-  /// c'tor for string values
-  ExprValue(const std::string& str)
-    : string_data(str) {
-    string_val.ptr = const_cast<char*>(string_data.data());
+  void Init(const std::string& str) {
+    string_data = str;
+    string_val.ptr = &string_data[0];
     string_val.len = string_data.size();
   }
 
@@ -198,6 +197,8 @@ struct ExprValue {
 
  private:
   std::string string_data; // Stores the data for string_val if necessary.
+
+  DISALLOW_COPY_AND_ASSIGN(ExprValue);
 };
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/9cee2b5f/be/src/exprs/literal.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/literal.cc b/be/src/exprs/literal.cc
index cc089da..1d816c3 100644
--- a/be/src/exprs/literal.cc
+++ b/be/src/exprs/literal.cc
@@ -74,7 +74,7 @@ Literal::Literal(const TExprNode& node)
     case TYPE_VARCHAR: {
       DCHECK_EQ(node.node_type, TExprNodeType::STRING_LITERAL);
       DCHECK(node.__isset.string_literal);
-      value_ = ExprValue(node.string_literal.value);
+      value_.Init(node.string_literal.value);
       if (type_.type == TYPE_VARCHAR) {
         value_.string_val.len = min(type_.len, value_.string_val.len);
       }
@@ -91,7 +91,7 @@ Literal::Literal(const TExprNode& node)
         // Pad out literal with spaces.
         str.replace(str_len, type_.len - str_len, type_.len - str_len, ' ');
       }
-      value_ = ExprValue(str);
+      value_.Init(str);
       break;
     }
     case TYPE_DECIMAL: {
@@ -186,16 +186,14 @@ Literal::Literal(ColumnType type, double v)
   }
 }
 
-Literal::Literal(ColumnType type, const string& v)
-  : Expr(type),
-    value_(v) {
+Literal::Literal(ColumnType type, const string& v) : Expr(type) {
+  value_.Init(v);
   DCHECK(type.type == TYPE_STRING || type.type == TYPE_CHAR || type.type == TYPE_VARCHAR)
       << type;
 }
 
-Literal::Literal(ColumnType type, const StringValue& v)
-  : Expr(type),
-    value_(v.DebugString()) {
+Literal::Literal(ColumnType type, const StringValue& v) : Expr(type) {
+  value_.Init(v.DebugString());
   DCHECK(type.type == TYPE_STRING || type.type == TYPE_CHAR) << type;
 }