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 2017/09/25 06:28:53 UTC

[1/3] incubator-impala git commit: IMPALA-4513: Promote integer types for ABS()

Repository: incubator-impala
Updated Branches:
  refs/heads/master 495325440 -> 646920810


IMPALA-4513: Promote integer types for ABS()

The internal representation of the most negative number
in two's complement requires 1 more bit to represent the
positive version.  This means ABS() must promote integer
types to the next highest width.

Change-Id: I86cc880e78258d5f90471bd8af4caeb4305eed77
Reviewed-on: http://gerrit.cloudera.org:8080/8004
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/f53ce3b1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/f53ce3b1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/f53ce3b1

Branch: refs/heads/master
Commit: f53ce3b16d476214167a686ecb513c6d866105b9
Parents: 4953254
Author: Zachary Amsden <za...@cloudera.com>
Authored: Thu Sep 7 14:15:19 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Sep 23 02:41:32 2017 +0000

----------------------------------------------------------------------
 be/src/exprs/expr-test.cc                       | 17 +++++++++++++++-
 be/src/exprs/math-functions-ir.cc               | 21 ++++++++++++++++----
 be/src/exprs/math-functions.h                   |  6 +++---
 common/function-registry/impala_functions.py    |  6 +++---
 .../queries/QueryTest/exprs.test                |  4 ++--
 5 files changed, 41 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index 33d77e0..e4d633d 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -4253,6 +4253,21 @@ TEST_F(ExprTest, MathFunctions) {
   TestValue("e()", TYPE_DOUBLE, M_E);
   TestValue("abs(cast(-1.0 as double))", TYPE_DOUBLE, 1.0);
   TestValue("abs(cast(1.0 as double))", TYPE_DOUBLE, 1.0);
+  TestValue("abs(-127)", TYPE_SMALLINT, 127);
+  TestValue("abs(-128)", TYPE_SMALLINT, 128);
+  TestValue("abs(127)", TYPE_SMALLINT, 127);
+  TestValue("abs(128)", TYPE_INT, 128);
+  TestValue("abs(-32767)", TYPE_INT, 32767);
+  TestValue("abs(-32768)", TYPE_INT, 32768);
+  TestValue("abs(32767)", TYPE_INT, 32767);
+  TestValue("abs(32768)", TYPE_BIGINT, 32768);
+  TestValue("abs(-1 * cast(pow(2, 31) as int))", TYPE_BIGINT, 2147483648);
+  TestValue("abs(cast(pow(2, 31) as int))", TYPE_BIGINT, 2147483648);
+  TestValue("abs(2147483647)", TYPE_BIGINT, 2147483647);
+  TestValue("abs(2147483647)", TYPE_BIGINT, 2147483647);
+  TestValue("abs(-9223372036854775807)", TYPE_BIGINT,  9223372036854775807);
+  TestValue("abs(9223372036854775807)", TYPE_BIGINT,  9223372036854775807);
+  TestIsNull("abs(-9223372036854775808)", TYPE_BIGINT);
   TestValue("sign(0.0)", TYPE_FLOAT, 0.0f);
   TestValue("sign(-0.0)", TYPE_FLOAT, 0.0f);
   TestValue("sign(+0.0)", TYPE_FLOAT, 0.0f);
@@ -4521,7 +4536,7 @@ TEST_F(ExprTest, MathFunctions) {
 
   // NULL arguments. In some cases the NULL can match multiple overloads so the result
   // type depends on the order in which function overloads are considered.
-  TestIsNull("abs(NULL)", TYPE_TINYINT);
+  TestIsNull("abs(NULL)", TYPE_SMALLINT);
   TestIsNull("sign(NULL)", TYPE_FLOAT);
   TestIsNull("exp(NULL)", TYPE_DOUBLE);
   TestIsNull("ln(NULL)", TYPE_DOUBLE);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/be/src/exprs/math-functions-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/math-functions-ir.cc b/be/src/exprs/math-functions-ir.cc
index 48b98a9..cc86d5b 100644
--- a/be/src/exprs/math-functions-ir.cc
+++ b/be/src/exprs/math-functions-ir.cc
@@ -59,12 +59,25 @@ DoubleVal MathFunctions::E(FunctionContext* ctx) {
     return RET_TYPE(FN(v1.val, v2.val)); \
   }
 
-ONE_ARG_MATH_FN(Abs, BigIntVal, BigIntVal, llabs);
+// N.B. - for integer math, we have to promote ABS() to the next highest integer type
+// because in two's complement arithmetic, the largest negative value for any bit width
+// is not representable as a positive value within the same width.  For the largest width,
+// we simply overflow.  In the unlikely event a workaround is needed, one can simply
+// cast to a higher precision decimal type.
+BigIntVal MathFunctions::Abs(FunctionContext* ctx, const BigIntVal& v) {
+  if (v.is_null) return BigIntVal::null();
+  if (UNLIKELY(v.val == std::numeric_limits<BigIntVal::underlying_type_t>::min())) {
+      ctx->AddWarning("abs() overflowed, returning NULL");
+      return BigIntVal::null();
+  }
+  return BigIntVal(llabs(v.val));
+}
+
+ONE_ARG_MATH_FN(Abs, BigIntVal, IntVal, llabs);
+ONE_ARG_MATH_FN(Abs, IntVal, SmallIntVal, abs);
+ONE_ARG_MATH_FN(Abs, SmallIntVal, TinyIntVal, abs);
 ONE_ARG_MATH_FN(Abs, DoubleVal, DoubleVal, fabs);
 ONE_ARG_MATH_FN(Abs, FloatVal, FloatVal, fabs);
-ONE_ARG_MATH_FN(Abs, IntVal, IntVal, abs);
-ONE_ARG_MATH_FN(Abs, SmallIntVal, SmallIntVal, abs);
-ONE_ARG_MATH_FN(Abs, TinyIntVal, TinyIntVal, abs);
 ONE_ARG_MATH_FN(Sin, DoubleVal, DoubleVal, sin);
 ONE_ARG_MATH_FN(Asin, DoubleVal, DoubleVal, asin);
 ONE_ARG_MATH_FN(Cos, DoubleVal, DoubleVal, cos);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/be/src/exprs/math-functions.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/math-functions.h b/be/src/exprs/math-functions.h
index 867ca2b..7063907 100644
--- a/be/src/exprs/math-functions.h
+++ b/be/src/exprs/math-functions.h
@@ -48,9 +48,9 @@ class MathFunctions {
   static BigIntVal Abs(FunctionContext*, const BigIntVal&);
   static DoubleVal Abs(FunctionContext*, const DoubleVal&);
   static FloatVal Abs(FunctionContext*, const FloatVal&);
-  static IntVal Abs(FunctionContext*, const IntVal&);
-  static SmallIntVal Abs(FunctionContext*, const SmallIntVal&);
-  static TinyIntVal Abs(FunctionContext*, const TinyIntVal&);
+  static BigIntVal Abs(FunctionContext*, const IntVal&);
+  static IntVal Abs(FunctionContext*, const SmallIntVal&);
+  static SmallIntVal Abs(FunctionContext*, const TinyIntVal&);
   static DoubleVal Sin(FunctionContext*, const DoubleVal&);
   static DoubleVal Asin(FunctionContext*, const DoubleVal&);
   static DoubleVal Cos(FunctionContext*, const DoubleVal&);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/common/function-registry/impala_functions.py
----------------------------------------------------------------------
diff --git a/common/function-registry/impala_functions.py b/common/function-registry/impala_functions.py
index c004775..0e3a3b8 100644
--- a/common/function-registry/impala_functions.py
+++ b/common/function-registry/impala_functions.py
@@ -260,9 +260,9 @@ visible_functions = [
   [['abs'], 'BIGINT', ['BIGINT'], 'impala::MathFunctions::Abs'],
   [['abs'], 'DOUBLE', ['DOUBLE'], 'impala::MathFunctions::Abs'],
   [['abs'], 'FLOAT', ['FLOAT'], 'impala::MathFunctions::Abs'],
-  [['abs'], 'INT', ['INT'], 'impala::MathFunctions::Abs'],
-  [['abs'], 'SMALLINT', ['SMALLINT'], 'impala::MathFunctions::Abs'],
-  [['abs'], 'TINYINT', ['TINYINT'], 'impala::MathFunctions::Abs'],
+  [['abs'], 'BIGINT', ['INT'], 'impala::MathFunctions::Abs'],
+  [['abs'], 'INT', ['SMALLINT'], 'impala::MathFunctions::Abs'],
+  [['abs'], 'SMALLINT', ['TINYINT'], 'impala::MathFunctions::Abs'],
   [['sign'], 'FLOAT', ['DOUBLE'], 'impala::MathFunctions::Sign'],
   [['sin'], 'DOUBLE', ['DOUBLE'], 'impala::MathFunctions::Sin'],
   [['asin'], 'DOUBLE', ['DOUBLE'], 'impala::MathFunctions::Asin'],

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/testdata/workloads/functional-query/queries/QueryTest/exprs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
index 9b1a1a7..92854b1 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -1866,7 +1866,7 @@ Infinity,NaN,-0,-Infinity,Nan
 double,double,double,double,double
 ====
 ---- QUERY
-# Test that abs() retains the type of the paramter IMPALA-1424
+# Test that abs() promotes the type of the paramter IMPALA-4513
 select abs(cast(1 as int)), abs(cast(1 as smallint)),
   abs(cast(1 as tinyint)), abs(cast(8589934592 as bigint)),
   abs(cast(-1.3 as double)), abs(cast(-1.3 as float)),
@@ -1874,7 +1874,7 @@ select abs(cast(1 as int)), abs(cast(1 as smallint)),
 ---- RESULTS
 1,1,1,8589934592,1.3,1.299999952316284,1.322
 ---- TYPES
-int, smallint, tinyint, bigint, double, float, decimal
+bigint, int, smallint, bigint, double, float, decimal
 ====
 ---- QUERY
 # Regression test for IMPALA-1508


[3/3] incubator-impala git commit: IMPALA-1767 Adds predicate to test boolean values true, false, unknown.

Posted by ta...@apache.org.
IMPALA-1767 Adds predicate to test boolean values true, false, unknown.

Adds a new expression to represent the following boolean predicate:
<expr> IS [NOT] (TRUE | FALSE | UNKNOWN)
The expression is expanded in the parser to istrue/false for the checks
against true and false respectively and to isnull for the check against
unknown. Compared to the other approaches (rewrites, extended backend expr),
this change is the simplest. Main downside is that error messages are
in terms of the lowered expression.

Testing:
- fe: parser, tosql, analyze exprs
- e2e: query exprs

Change-Id: I9d5fba65ef6c87dfc55a25d2c45246f74eb48c40
Reviewed-on: http://gerrit.cloudera.org:8080/8122
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/64692081
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/64692081
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/64692081

Branch: refs/heads/master
Commit: 646920810f581698c5150553d6bccc0f82c0fe6b
Parents: d53f43b
Author: Vuk Ercegovac <ve...@cloudera.com>
Authored: Thu Sep 21 13:47:37 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Sep 23 04:33:20 2017 +0000

----------------------------------------------------------------------
 be/src/exprs/expr-test.cc                       | 42 +++++++++++++++++++
 fe/src/main/cup/sql-parser.cup                  | 34 ++++++++++++++-
 .../impala/analysis/AnalyzeExprsTest.java       | 40 ++++++++++++++++++
 .../org/apache/impala/analysis/ParserTest.java  | 22 +++++++---
 .../org/apache/impala/analysis/ToSqlTest.java   | 10 +++++
 .../queries/QueryTest/exprs.test                | 44 ++++++++++++++++++++
 6 files changed, 184 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/64692081/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index e4d633d..6a63ad5 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -2548,6 +2548,48 @@ TEST_F(ExprTest, IsNullPredicate) {
   TestValue("NULL IS NOT NULL", TYPE_BOOLEAN, false);
 }
 
+TEST_F(ExprTest, BoolTestExpr) {
+  // Tests against constants.
+  TestValue("TRUE IS TRUE", TYPE_BOOLEAN, true);
+  TestValue("TRUE IS FALSE", TYPE_BOOLEAN, false);
+  TestValue("TRUE IS UNKNOWN", TYPE_BOOLEAN, false);
+  TestValue("TRUE IS NOT TRUE", TYPE_BOOLEAN, false);
+  TestValue("TRUE IS NOT FALSE", TYPE_BOOLEAN, true);
+  TestValue("TRUE IS NOT UNKNOWN", TYPE_BOOLEAN, true);
+  TestValue("FALSE IS TRUE", TYPE_BOOLEAN, false);
+  TestValue("FALSE IS FALSE", TYPE_BOOLEAN, true);
+  TestValue("FALSE IS UNKNOWN", TYPE_BOOLEAN, false);
+  TestValue("FALSE IS NOT TRUE", TYPE_BOOLEAN, true);
+  TestValue("FALSE IS NOT FALSE", TYPE_BOOLEAN, false);
+  TestValue("FALSE IS NOT UNKNOWN", TYPE_BOOLEAN, true);
+  TestValue("NULL IS TRUE", TYPE_BOOLEAN, false);
+  TestValue("NULL IS FALSE", TYPE_BOOLEAN, false);
+  TestValue("NULL IS UNKNOWN", TYPE_BOOLEAN, true);
+  TestValue("NULL IS NOT TRUE", TYPE_BOOLEAN, true);
+  TestValue("NULL IS NOT FALSE", TYPE_BOOLEAN, true);
+  TestValue("NULL IS NOT UNKNOWN", TYPE_BOOLEAN, false);
+
+  // Tests against expressions
+  TestValue("(2>1) IS TRUE", TYPE_BOOLEAN, true);
+  TestValue("(2>1) IS FALSE", TYPE_BOOLEAN, false);
+  TestValue("(2>1) IS UNKNOWN", TYPE_BOOLEAN, false);
+  TestValue("(2>1) IS NOT TRUE", TYPE_BOOLEAN, false);
+  TestValue("(2>1) IS NOT FALSE", TYPE_BOOLEAN, true);
+  TestValue("(2>1) IS NOT UNKNOWN", TYPE_BOOLEAN, true);
+  TestValue("(1>2) IS TRUE", TYPE_BOOLEAN, false);
+  TestValue("(1>2) IS FALSE", TYPE_BOOLEAN, true);
+  TestValue("(1>2) IS UNKNOWN", TYPE_BOOLEAN, false);
+  TestValue("(1>2) IS NOT TRUE", TYPE_BOOLEAN, true);
+  TestValue("(1>2) IS NOT FALSE", TYPE_BOOLEAN, false);
+  TestValue("(1>2) IS NOT UNKNOWN", TYPE_BOOLEAN, true);
+  TestValue("(NULL = 1) IS TRUE", TYPE_BOOLEAN, false);
+  TestValue("(NULL = 1) IS FALSE", TYPE_BOOLEAN, false);
+  TestValue("(NULL = 1) IS UNKNOWN", TYPE_BOOLEAN, true);
+  TestValue("(NULL = 1) IS NOT TRUE", TYPE_BOOLEAN, true);
+  TestValue("(NULL = 1) IS NOT FALSE", TYPE_BOOLEAN, true);
+  TestValue("(NULL = 1) IS NOT UNKNOWN", TYPE_BOOLEAN, false);
+}
+
 TEST_F(ExprTest, LikePredicate) {
   TestValue("'a' LIKE '%a%'", TYPE_BOOLEAN, true);
   TestValue("'a' LIKE '%abcde'", TYPE_BOOLEAN, false);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/64692081/fe/src/main/cup/sql-parser.cup
----------------------------------------------------------------------
diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup
index 0eaa68e..7c0a794 100644
--- a/fe/src/main/cup/sql-parser.cup
+++ b/fe/src/main/cup/sql-parser.cup
@@ -339,8 +339,9 @@ nonterminal ArrayList<String> opt_ident_list, opt_sort_cols;
 nonterminal TableName table_name;
 nonterminal FunctionName function_name;
 nonterminal Expr where_clause;
-nonterminal Predicate predicate, between_predicate, comparison_predicate,
-  compound_predicate, in_predicate, like_predicate, exists_predicate;
+nonterminal Expr predicate, bool_test_expr;
+nonterminal Predicate between_predicate, comparison_predicate, compound_predicate,
+  in_predicate, like_predicate, exists_predicate;
 nonterminal ArrayList<Expr> group_by_clause, opt_partition_by_clause;
 nonterminal Expr having_clause;
 nonterminal ArrayList<OrderByElement> order_by_elements, opt_order_by_clause;
@@ -493,6 +494,7 @@ nonterminal Boolean server_ident;
 nonterminal Boolean source_ident;
 nonterminal Boolean sources_ident;
 nonterminal Boolean uri_ident;
+nonterminal Boolean unknown_ident;
 
 // For Create/Drop/Show function ddl
 nonterminal FunctionArgs function_def_args;
@@ -1678,6 +1680,16 @@ option_ident ::=
   :}
   ;
 
+unknown_ident ::=
+  IDENT:ident
+  {:
+    if (!ident.toUpperCase().equals("UNKNOWN")) {
+      parser.parseError("identifier", SqlParserSymbols.IDENT, "UNKNOWN");
+    }
+    RESULT = true;
+  :}
+  ;
+
 view_column_defs ::=
   LPAREN view_column_def_list:view_col_defs RPAREN
   {: RESULT = view_col_defs; :}
@@ -2933,6 +2945,8 @@ predicate ::=
   {: RESULT = p; :}
   | like_predicate:p
   {: RESULT = p; :}
+  | bool_test_expr:e
+  {: RESULT = e; :}
   | LPAREN predicate:p RPAREN
   {:
     p.setPrintSqlInParens(true);
@@ -3017,6 +3031,22 @@ in_predicate ::=
   {: RESULT = new InPredicate(e, s, true); :}
   ;
 
+// Boolean test expression: <expr> IS [NOT] (TRUE | FALSE | UNKNOWN)
+bool_test_expr ::=
+  expr:e KW_IS KW_TRUE
+  {: RESULT = new FunctionCallExpr("istrue", Lists.newArrayList(e)); :}
+  | expr:e KW_IS KW_NOT KW_TRUE
+  {: RESULT = new FunctionCallExpr("isnottrue", Lists.newArrayList(e)); :}
+  | expr:e KW_IS KW_FALSE
+  {: RESULT = new FunctionCallExpr("isfalse", Lists.newArrayList(e)); :}
+  | expr:e KW_IS KW_NOT KW_FALSE
+  {: RESULT = new FunctionCallExpr("isnotfalse", Lists.newArrayList(e)); :}
+  | expr:e KW_IS unknown_ident
+  {: RESULT = new IsNullPredicate(e, false); :}
+  | expr:e KW_IS KW_NOT unknown_ident
+  {: RESULT = new IsNullPredicate(e, true); :}
+  ;
+
 subquery ::=
   LPAREN subquery:s RPAREN
   {: RESULT = s; :}

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/64692081/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
index e393f65..8891fee 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -613,6 +613,7 @@ public class AnalyzeExprsTest extends AnalyzerTest {
     AnalyzesOk("select * from functional.alltypes where int_col is null");
     AnalyzesOk("select * from functional.alltypes where string_col is not null");
     AnalyzesOk("select * from functional.alltypes where null is not null");
+
     AnalysisError("select 1 from functional.allcomplextypes where int_map_col is null",
         "IS NULL predicate does not support complex types: int_map_col IS NULL");
     AnalysisError("select * from functional.allcomplextypes where complex_struct_col " +
@@ -624,6 +625,45 @@ public class AnalyzeExprsTest extends AnalyzerTest {
   }
 
   @Test
+  public void TestBoolTestExpression() throws AnalysisException {
+    String[] rhsOptions = new String[] {"true", "false", "unknown"};
+    String[] lhsOptions = new String[] {
+        "bool_col",      // column reference
+        "1>1",           // boolean expression
+        "istrue(false)", // function
+        "(1>1 is true)"  // nested expression
+        };
+
+    // Bool test in both the select and where clauses. Includes negated IS clauses.
+    for (String rhsVal : rhsOptions) {
+      String template = "select (%s is %s) from functional.alltypes where (%s is %s)";
+      String lhsVal = rhsVal == "unknown" ? "null" : rhsVal;
+      String negatedRhsVal = "not " + rhsVal;
+
+      // Tests for lhs being one of true, false or null.
+      AnalyzesOk(String.format(template, lhsVal, rhsVal, lhsVal, rhsVal));
+      AnalyzesOk(String.format(template, lhsVal, negatedRhsVal, lhsVal, negatedRhsVal));
+
+      // Tests for lhs being one lhsOptions expressions.
+      for (String lhs : lhsOptions) {
+        AnalyzesOk(String.format(template, lhs, rhsVal, lhs, rhsVal));
+        AnalyzesOk(String.format(template, lhs, negatedRhsVal, lhs, negatedRhsVal));
+      }
+    }
+
+    AnalysisError("select ('foo' is true)",
+        "No matching function with signature: istrue(STRING).");
+    AnalysisError("select (10 is true)",
+        "No matching function with signature: istrue(TINYINT).");
+    AnalysisError("select (10 is not true)",
+        "No matching function with signature: isnottrue(TINYINT).");
+    AnalysisError("select (10 is false)",
+        "No matching function with signature: isfalse(TINYINT).");
+    AnalysisError("select (10 is not false)",
+        "No matching function with signature: isnotfalse(TINYINT).");
+  }
+
+  @Test
   public void TestBetweenPredicates() throws AnalysisException {
     AnalyzesOk("select * from functional.alltypes " +
         "where tinyint_col between smallint_col and int_col");

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/64692081/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
index d6d808f..1f9a6af 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
@@ -1446,17 +1446,27 @@ public class ParserTest extends FrontendTestBase {
     operations.add("regexp");
     operations.add("iregexp");
 
-    for (String lop: operands_) {
-      for (String rop: operands_) {
+    ArrayList<String> boolTestVals = new ArrayList<String>();
+    boolTestVals.add("null");
+    boolTestVals.add("unknown");
+    boolTestVals.add("true");
+    boolTestVals.add("false");
+
+    for (String lop : operands_) {
+      for (String rop : operands_) {
         for (String op : operations) {
           String expr = String.format("%s %s %s", lop, op.toString(), rop);
           ParsesOk(String.format("select %s from t where %s", expr, expr));
         }
       }
-      String isNullExr = String.format("%s is null", lop);
-      String isNotNullExr = String.format("%s is not null", lop);
-      ParsesOk(String.format("select %s from t where %s", isNullExr, isNullExr));
-      ParsesOk(String.format("select %s from t where %s", isNotNullExr, isNotNullExr));
+      for (String val : boolTestVals) {
+        String isExpr = String.format("%s is %s", lop, val);
+        String isNotExpr = String.format("%s is not %s", lop, val);
+        ParsesOk(String.format("select %s from t where %s", isExpr, isExpr));
+        ParsesOk(String.format("select %s from t where %s", isNotExpr, isNotExpr));
+      }
+      ParserError(String.format("select %s is nonsense", lop));
+      ParserError(String.format("select %s is not nonsense", lop));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/64692081/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
index 4cd8934..d70d7ee 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
@@ -1254,6 +1254,16 @@ public class ToSqlTest extends FrontendTestBase {
     // IsNullPredicate.
     testToSql("select 5 is null, (5 is null), 10 is not null, (10 is not null)",
         "SELECT 5 IS NULL, (5 IS NULL), 10 IS NOT NULL, (10 IS NOT NULL)");
+    // Boolean test expression (expanded to istrue/false).
+    testToSql("select (true is true)", "SELECT (istrue(TRUE))");
+    testToSql("select (true is not true)", "SELECT (isnottrue(TRUE))");
+    testToSql("select (true is false)", "SELECT (isfalse(TRUE))");
+    testToSql("select (true is unknown)", "SELECT (TRUE IS NULL)");
+    testToSql("select (true is not unknown)", "SELECT (TRUE IS NOT NULL)");
+    testToSql("select not(true is true)", "SELECT NOT (istrue(TRUE))");
+    testToSql("select (false is false)", "SELECT (isfalse(FALSE))");
+    testToSql("select (null is unknown)", "SELECT (NULL IS NULL)");
+    testToSql("select (1 > 1 is true is unknown)", "SELECT (istrue(1 > 1) IS NULL)");
     // LikePredicate.
     testToSql("select 'a' LIKE '%b.', ('a' LIKE '%b.'), " +
         "'a' ILIKE '%b.', ('a' ILIKE '%b.'), " +

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/64692081/testdata/workloads/functional-query/queries/QueryTest/exprs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
index 92854b1..811a169 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -71,6 +71,50 @@ select count(*) from alltypesagg where tinyint_col is not null
 bigint
 ====
 ---- QUERY
+# boolean test: IS [NOT] (TRUE | FALSE | UNKNOWN)
+---- QUERY
+select count(*) from alltypesagg where (int_col < 100) is unknown;
+---- RESULTS
+20
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from alltypesagg where (int_col < 100) is true;
+---- RESULTS
+1080
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from alltypesagg where (int_col < 100) is false;
+---- RESULTS
+9900
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from alltypesagg where (int_col < 100) is not unknown;
+---- RESULTS
+10980
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from alltypesagg where (int_col < 100) is not true;
+---- RESULTS
+9920
+---- TYPES
+bigint
+====
+---- QUERY
+select count(*) from alltypesagg where (int_col < 100) is not false;
+---- RESULTS
+1100
+---- TYPES
+bigint
+====
+---- QUERY
 # =
 select count(*) from alltypesagg where tinyint_col = 1
 ---- RESULTS


[2/3] incubator-impala git commit: IMPALA-5599: Fix for mis-use of TimestampValue

Posted by ta...@apache.org.
IMPALA-5599: Fix for mis-use of TimestampValue

The TimestampValue class is being used for non-database purposes
in many places, such as in log messages.

This change proposes to introduce APIs to convert Unix timetamps
into the corresponding date-time strings. We provide a series of
functions for different input time units, and also give the user
control over the precision of the output date-time string. APIs
are provided to format in UTC and local time zones. The new APIs
can be used to replace (or instead of) TimestampValue::ToString()
in those places where Unix timestamps are being converted to
strings for printing.

The current commit implements the APIs and replaces calls to
TimestampValue::ToString() in be/src/service.

A new unit test, time-test, has been added to the back-end tests.

Other uses of TimestampValue in be/src/service, such as to track
start and end times of queries, etc., will be analyzed and changed
as appropriate in a follow-up commit.

Change-Id: I9b0ae06f6d94968c87a199625aa3332b26988142
Reviewed-on: http://gerrit.cloudera.org:8080/8084
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/d53f43b4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/d53f43b4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/d53f43b4

Branch: refs/heads/master
Commit: d53f43b42c7c41f72d6a31505344f0393bdbe8a5
Parents: f53ce3b
Author: Zoram Thanga <zo...@cloudera.com>
Authored: Fri Sep 15 15:27:44 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Sep 23 02:46:01 2017 +0000

----------------------------------------------------------------------
 be/src/service/impala-server.cc |  19 +++---
 be/src/util/CMakeLists.txt      |   1 +
 be/src/util/time-test.cc        | 113 +++++++++++++++++++++++++++++++++++
 be/src/util/time.cc             | 108 +++++++++++++++++++++++++++++++++
 be/src/util/time.h              |  46 +++++++++++++-
 5 files changed, 274 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d53f43b4/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 051cfaf..11de619 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -75,6 +75,7 @@
 #include "util/summary-util.h"
 #include "util/test-info.h"
 #include "util/uid-util.h"
+#include "util/time.h"
 
 #include "gen-cpp/Types_types.h"
 #include "gen-cpp/ImpalaService.h"
@@ -914,12 +915,10 @@ Status ImpalaServer::ExecuteInternal(
 
 void ImpalaServer::PrepareQueryContext(TQueryCtx* query_ctx) {
   query_ctx->__set_pid(getpid());
-  TimestampValue utc_timestamp = TimestampValue::UtcTime();
-  query_ctx->__set_utc_timestamp_string(utc_timestamp.ToString());
-  TimestampValue local_timestamp(utc_timestamp);
-  local_timestamp.UtcToLocal();
-  query_ctx->__set_now_string(local_timestamp.ToString());
-  query_ctx->__set_start_unix_millis(UnixMillis());
+  int64_t now_us = UnixMicros();
+  query_ctx->__set_utc_timestamp_string(ToUtcStringFromUnixMicros(now_us));
+  query_ctx->__set_now_string(ToStringFromUnixMicros(now_us));
+  query_ctx->__set_start_unix_millis(now_us / MICROS_PER_MILLI);
   query_ctx->__set_coord_address(ExecEnv::GetInstance()->backend_address());
 
   // Creating a random_generator every time is not free, but
@@ -1141,11 +1140,10 @@ Status ImpalaServer::GetSessionState(const TUniqueId& session_id,
     if (mark_active) {
       lock_guard<mutex> session_lock(i->second->lock);
       if (i->second->expired) {
-        int64_t last_time_s = i->second->last_accessed_ms / 1000;
         stringstream ss;
         ss << "Client session expired due to more than " << i->second->session_timeout
            << "s of inactivity (last activity was at: "
-           << TimestampValue::FromUnixTime(last_time_s).ToString() << ").";
+           << ToStringFromUnixMillis(i->second->last_accessed_ms) << ").";
         return Status(ss.str());
       }
       if (i->second->closed) return Status("Session is closed");
@@ -1843,7 +1841,7 @@ void ImpalaServer::RegisterSessionTimeout(int32_t session_timeout) {
         if (now - last_accessed_ms <= session_timeout_ms) continue;
         LOG(INFO) << "Expiring session: " << session_state.first << ", user:"
                   << session_state.second->connected_user << ", last active: "
-                  << TimestampValue::FromUnixTime(last_accessed_ms / 1000).ToString();
+                  << ToStringFromUnixMillis(last_accessed_ms);
         session_state.second->expired = true;
         ImpaladMetrics::NUM_SESSIONS_EXPIRED->Increment(1L);
         // Since expired is true, no more queries will be added to the inflight list.
@@ -1919,11 +1917,10 @@ void ImpalaServer::RegisterSessionTimeout(int32_t session_timeout) {
           }
         } else if (!query_state->is_active()) {
           // Otherwise time to expire this query
-          int64_t last_active_s = query_state->last_active_ms() / 1000;
           VLOG_QUERY
               << "Expiring query due to client inactivity: " << expiration_event->second
               << ", last activity was at: "
-              << TimestampValue::FromUnixTime(last_active_s).ToString();
+              << ToStringFromUnixMillis(query_state->last_active_ms());
           const string& err_msg = Substitute(
               "Query $0 expired due to client inactivity (timeout is $1)",
               PrintId(expiration_event->second),

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d53f43b4/be/src/util/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/be/src/util/CMakeLists.txt b/be/src/util/CMakeLists.txt
index 3f18094..947ab78 100644
--- a/be/src/util/CMakeLists.txt
+++ b/be/src/util/CMakeLists.txt
@@ -133,5 +133,6 @@ ADD_BE_TEST(runtime-profile-test)
 ADD_BE_TEST(string-parser-test)
 ADD_BE_TEST(symbols-util-test)
 ADD_BE_TEST(thread-pool-test)
+ADD_BE_TEST(time-test)
 ADD_BE_TEST(uid-util-test)
 ADD_BE_TEST(webserver-test)

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d53f43b4/be/src/util/time-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/time-test.cc b/be/src/util/time-test.cc
new file mode 100644
index 0000000..dde0d12
--- /dev/null
+++ b/be/src/util/time-test.cc
@@ -0,0 +1,113 @@
+// 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.
+
+#include <algorithm>
+#include <regex>
+#include <sstream>
+
+#include "util/time.h"
+#include "testutil/gtest-util.h"
+
+#include "common/names.h"
+
+using namespace std;
+
+namespace impala {
+  // Basic tests for the time formatting APIs in util/time.h.
+TEST(TimeTest, Basic) {
+  EXPECT_EQ("1970-01-01 00:00:00", ToUtcStringFromUnix(0));
+  EXPECT_EQ("1970-01-01 00:00:00.000", ToUtcStringFromUnixMillis(0));
+  EXPECT_EQ("1970-01-01 00:00:00.001", ToUtcStringFromUnixMillis(1));
+  EXPECT_EQ("1970-01-01 00:00:00", ToUtcStringFromUnixMillis(0, TimePrecision::Second));
+  EXPECT_EQ("1970-01-01 00:00:00.000000", ToUtcStringFromUnixMicros(0));
+  EXPECT_EQ("1970-01-01 00:00:00.000001", ToUtcStringFromUnixMicros(1));
+  EXPECT_EQ("1970-01-01 00:00:00.000", ToUtcStringFromUnixMicros(0,
+      TimePrecision::Millisecond));
+  EXPECT_EQ("1970-01-01 00:00:00", ToUtcStringFromUnixMicros(0, TimePrecision::Second));
+
+  EXPECT_EQ("1970-01-01 00:01:00", ToUtcStringFromUnix(60));
+  EXPECT_EQ("1970-01-01 01:00:00", ToUtcStringFromUnix(3600));
+  // Check we are handling negative times - time before the Unix epoch
+  EXPECT_EQ("1969-12-31 23:59:59", ToUtcStringFromUnix(-1));
+
+  // The earliest date-time that can be represented with the high-resolution
+  // chrono::system_clock is 1677-09-21 00:12:44 UTC, which is
+  // -9223372036854775808 (INT64_MIN) nanoseconds before the Unix epoch.
+  EXPECT_EQ("1677-09-21 00:12:44",
+      ToUtcStringFromUnix(INT64_MIN / NANOS_PER_SEC));
+  EXPECT_EQ("1677-09-21 00:12:44.854",
+      ToUtcStringFromUnixMillis(INT64_MIN / NANOS_PER_MICRO / MICROS_PER_MILLI));
+  EXPECT_EQ("1677-09-21 00:12:44.854775",
+      ToUtcStringFromUnixMicros(INT64_MIN / NANOS_PER_MICRO));
+
+  // The latest date-time that can be represented with the high-resoliution
+  // chrono::system_clock is 2262-04-11 23:47:16 UTC, which is
+  // 9223372036854775807 (INT64_MAX) nanoseconds since the Unix epoch.
+  EXPECT_EQ("2262-04-11 23:47:16",
+      ToUtcStringFromUnix(INT64_MAX / NANOS_PER_SEC));
+  EXPECT_EQ("2262-04-11 23:47:16.854",
+      ToUtcStringFromUnixMillis(INT64_MAX / NANOS_PER_MICRO / MICROS_PER_MILLI));
+  EXPECT_EQ("2262-04-11 23:47:16.854775",
+      ToUtcStringFromUnixMicros(INT64_MAX / NANOS_PER_MICRO));
+
+  EXPECT_EQ("1969-12-31 23:59:59.000", ToUtcStringFromUnixMillis(-1000));
+  EXPECT_EQ("1969-12-31 23:59:59.999", ToUtcStringFromUnixMillis(-1999));
+  EXPECT_EQ("1969-12-31 23:59:58.000", ToUtcStringFromUnixMillis(-2000));
+  EXPECT_EQ("1969-12-31 23:59:58.001", ToUtcStringFromUnixMillis(-2001));
+
+  EXPECT_EQ("1969-12-31 23:59:59.000000", ToUtcStringFromUnixMicros(-1000000));
+  EXPECT_EQ("1969-12-31 23:59:59.999999", ToUtcStringFromUnixMicros(-1999999));
+  EXPECT_EQ("1969-12-31 23:59:58.000001", ToUtcStringFromUnixMicros(-2000001));
+
+  // Unix time does not represent leap seconds. Test continuous roll-over of
+  // Unix time after 1998-12-31 23:59:59
+  EXPECT_EQ("1998-12-31 23:59:59", ToUtcStringFromUnix(915148799));
+  EXPECT_EQ("1999-01-01 00:00:00", ToUtcStringFromUnix(915148800));
+
+  // Check that for the same Unix time, our output string agrees with the output
+  // of strftime(3).
+  int64_t now_s = UnixMillis() / MILLIS_PER_SEC;
+  char now_buf[256];
+  strftime(now_buf, sizeof(now_buf), "%F %T", localtime(static_cast<time_t *>(&now_s)));
+  EXPECT_EQ(string(now_buf), ToStringFromUnix(now_s)) << "now_s=" << now_s;
+
+  strftime(now_buf, sizeof(now_buf), "%F %T", gmtime(static_cast<time_t *>(&now_s)));
+  EXPECT_EQ(string(now_buf), ToUtcStringFromUnix(now_s)) << "now_s=" << now_s;
+
+  // Check zero-padding of date-time string's fractional second part if input
+  // time's resolution is less than that requested by the caller.
+  smatch sm; // Place holder to be passed to regex_search() below.
+  string s1 = ToStringFromUnix(now_s, TimePrecision::Millisecond);
+  string s2 = ToUtcStringFromUnix(now_s, TimePrecision::Millisecond);
+  EXPECT_TRUE(regex_search(s1, sm, regex(R"(\.(000)$)")));
+  EXPECT_TRUE(regex_search(s2, sm, regex(R"(\.(000)$)")));
+
+  int64_t now_ms = UnixMillis();
+  s1 = ToStringFromUnixMillis(now_ms, TimePrecision::Microsecond);
+  s2 = ToUtcStringFromUnixMillis(now_ms, TimePrecision::Microsecond);
+  EXPECT_TRUE(regex_search(s1, sm, regex(R"(\.\d{3}(000)$)")));
+  EXPECT_TRUE(regex_search(s2, sm, regex(R"(\.\d{3}(000)$)")));
+
+  int64_t now_us = UnixMicros();
+  s1 = ToStringFromUnixMicros(now_us, TimePrecision::Nanosecond);
+  s2 = ToUtcStringFromUnixMicros(now_us, TimePrecision::Nanosecond);
+  EXPECT_TRUE(regex_search(s1, sm, regex(R"(\.\d{6}(000)$)")));
+  EXPECT_TRUE(regex_search(s2, sm, regex(R"(\.\d{6}(000)$)")));
+} // TEST
+} // namespace impala
+
+IMPALA_TEST_MAIN();

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d53f43b4/be/src/util/time.cc
----------------------------------------------------------------------
diff --git a/be/src/util/time.cc b/be/src/util/time.cc
index e6530de..dd0587b 100644
--- a/be/src/util/time.cc
+++ b/be/src/util/time.cc
@@ -17,6 +17,9 @@
 
 #include <chrono>
 #include <thread>
+#include <iomanip>
+#include <sstream>
+#include <cstdlib>
 
 #include "util/time.h"
 
@@ -26,3 +29,108 @@ using namespace std;
 void impala::SleepForMs(const int64_t duration_ms) {
   this_thread::sleep_for(chrono::milliseconds(duration_ms));
 }
+
+// Convert the given time_point, 't', into a date-time string in the
+// UTC time zone if 'utc' is true, or the local time zone if it is false.
+// The returned string is of the form yyy-MM-dd HH::mm::SS.
+static string TimepointToString(const chrono::system_clock::time_point& t,
+    bool utc) {
+  char buf[256];
+  struct tm tmp;
+  auto input_time = chrono::system_clock::to_time_t(t);
+
+  // gcc 4.9 does not support C++14 get_time and put_time functions, so we're
+  // stuck with strftime() for now.
+  if (utc) {
+    strftime(buf, sizeof(buf), "%F %T", gmtime_r(&input_time, &tmp));
+  } else {
+    strftime(buf, sizeof(buf), "%F %T", localtime_r(&input_time, &tmp));
+  }
+  return string(buf);
+}
+
+// Format the sub-second part of the input time point object 't', at the
+// precision specified by 'p'. The returned string is meant to be appended to
+// the string returned by TimePointToString() above.
+// Note the use of abs(). This is to make sure we correctly format negative times,
+// i.e., times before the Unix epoch.
+static string FormatSubSecond(const chrono::system_clock::time_point& t,
+    TimePrecision p) {
+  stringstream ss;
+  auto frac = t.time_since_epoch();
+  if (p == TimePrecision::Millisecond) {
+    auto subsec = chrono::duration_cast<chrono::milliseconds>(frac) % MILLIS_PER_SEC;
+    ss << "." << std::setfill('0') << std::setw(3) << abs(subsec.count());
+  } else if (p == TimePrecision::Microsecond) {
+    auto subsec = chrono::duration_cast<chrono::microseconds>(frac) % MICROS_PER_SEC;
+    ss << "." << std::setfill('0') << std::setw(6) << abs(subsec.count());
+  } else if (p == TimePrecision::Nanosecond) {
+    auto subsec = chrono::duration_cast<chrono::nanoseconds>(frac) % NANOS_PER_SEC;
+    ss << "." << std::setfill('0') << std::setw(9) << abs(subsec.count());
+  } else {
+    // 1-second precision or unknown unit. Return empty string.
+    DCHECK_EQ(TimePrecision::Second, p);
+    ss << "";
+  }
+  return ss.str();
+}
+
+// Convert time point 't' into date-time string at precision 'p'.
+// Output string is in UTC time zone if 'utc' is true, else it is in the
+// local time zone.
+static string ToString(const chrono::system_clock::time_point& t, TimePrecision p,
+    bool utc)
+{
+  stringstream ss;
+  ss << TimepointToString(t, utc);
+  ss << FormatSubSecond(t, p);
+  return ss.str();
+}
+
+// Convenience function to convert Unix time, specified as seconds since
+// the Unix epoch, into a C++ time_point object.
+static chrono::system_clock::time_point TimepointFromUnix(int64_t s) {
+  return chrono::system_clock::time_point(chrono::seconds(s));
+}
+
+// Convenience function to convert Unix time, specified as milliseconds since
+// the Unix epoch, into a C++ time_point object.
+static chrono::system_clock::time_point TimepointFromUnixMillis(int64_t ms) {
+  return chrono::system_clock::time_point(chrono::milliseconds(ms));
+}
+
+// Convenience function to convert Unix time, specified as microseconds since
+// the Unix epoch, into a C++ time_point object.
+static chrono::system_clock::time_point TimepointFromUnixMicros(int64_t us) {
+  return chrono::system_clock::time_point(chrono::microseconds(us));
+}
+
+string impala::ToStringFromUnix(int64_t s, TimePrecision p) {
+  chrono::system_clock::time_point t = TimepointFromUnix(s);
+  return ToString(t, p, false);
+}
+
+string impala::ToUtcStringFromUnix(int64_t s, TimePrecision p) {
+  chrono::system_clock::time_point t = TimepointFromUnix(s);
+  return ToString(t, p, true);
+}
+
+string impala::ToStringFromUnixMillis(int64_t ms, TimePrecision p) {
+  chrono::system_clock::time_point t = TimepointFromUnixMillis(ms);
+  return ToString(t, p, false);
+}
+
+string impala::ToUtcStringFromUnixMillis(int64_t ms, TimePrecision p) {
+  chrono::system_clock::time_point t = TimepointFromUnixMillis(ms);
+  return ToString(t, p, true);
+}
+
+string impala::ToStringFromUnixMicros(int64_t us, TimePrecision p) {
+  chrono::system_clock::time_point t = TimepointFromUnixMicros(us);
+  return ToString(t, p, false);
+}
+
+string impala::ToUtcStringFromUnixMicros(int64_t us, TimePrecision p) {
+  chrono::system_clock::time_point t = TimepointFromUnixMicros(us);
+  return ToString(t, p, true);
+}

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d53f43b4/be/src/util/time.h
----------------------------------------------------------------------
diff --git a/be/src/util/time.h b/be/src/util/time.h
index 3ec009a..5e9a4fd 100644
--- a/be/src/util/time.h
+++ b/be/src/util/time.h
@@ -20,6 +20,7 @@
 
 #include <stdint.h>
 #include <time.h>
+#include <string>
 
 #include "gutil/walltime.h"
 
@@ -48,7 +49,6 @@ inline int64_t MonotonicSeconds() {
   return GetMonoTimeMicros() / MICROS_PER_SEC;
 }
 
-
 /// Returns the number of milliseconds that have passed since the Unix epoch. This is
 /// affected by manual changes to the system clock but is more suitable for use across
 /// a cluster. For more accurate timings on the local host use the monotonic functions
@@ -57,9 +57,51 @@ inline int64_t UnixMillis() {
   return GetCurrentTimeMicros() / MICROS_PER_MILLI;
 }
 
+/// Returns the number of microseconds that have passed since the Unix epoch. This is
+/// affected by manual changes to the system clock but is more suitable for use across
+/// a cluster. For more accurate timings on the local host use the monotonic functions
+/// above.
+inline int64_t UnixMicros() {
+  return GetCurrentTimeMicros();
+}
+
 /// Sleeps the current thread for at least duration_ms milliseconds.
 void SleepForMs(const int64_t duration_ms);
 
-}
+// An enum class to use as precision argument for the ToString*() functions below
+enum TimePrecision {
+  Second,
+  Millisecond,
+  Microsecond,
+  Nanosecond
+};
+
+/// Converts the input Unix time, 's', specified in seconds since the Unix epoch, to a
+/// date-time string in the local time zone. The precision in the output date-time string
+/// is specified by the second argument, 'p'. The returned string is of the format
+/// yyyy-MM-dd HH:mm:SS[.ms[us[ns]]. It's worth noting that if the precision specified
+/// by 'p' is higher than that of the input timestamp, the part corresponding to
+/// 'p' in the fractional second part of the output will just be zero-padded.
+std::string ToStringFromUnix(int64_t s, TimePrecision p = TimePrecision::Second);
+
+/// Converts input seconds-since-epoch to date-time string in UTC time zone.
+std::string ToUtcStringFromUnix(int64_t s, TimePrecision p = TimePrecision::Second);
+
+/// Converts input milliseconds-since-epoch to date-time string in local time zone.
+std::string ToStringFromUnixMillis(int64_t ms,
+    TimePrecision p = TimePrecision::Millisecond);
+
+/// Converts input milliseconds-since-epoch to date-time string in UTC time zone.
+std::string ToUtcStringFromUnixMillis(int64_t ms,
+    TimePrecision p = TimePrecision::Millisecond);
+
+/// Converts input microseconds-since-epoch to date-time string in local time zone.
+std::string ToStringFromUnixMicros(int64_t us,
+    TimePrecision p = TimePrecision::Microsecond);
+
+/// Converts input microseconds-since-epoch to date-time string in UTC time zone.
+std::string ToUtcStringFromUnixMicros(int64_t us,
+    TimePrecision p = TimePrecision::Microsecond);
 
+} // namespace impala
 #endif