From b22b565a9268b8b64a683befc3ba7274a76cc348 Mon Sep 17 00:00:00 2001
From: Nong Li <nong@cloudera.com>
Date: Mon, 12 Nov 2012 11:33:14 -0800
Subject: [PATCH] Fix codegen for min/max of bool col.

---
 be/src/codegen/llvm-codegen.cc                | 27 ++++++++++++++-----
 be/src/exprs/expr.h                           |  2 +-
 .../queries/QueryTest/aggregation.test        | 16 ++++++++++-
 3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index 765e340c9..b750e73ef 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -627,6 +627,15 @@ Function* LlvmCodeGen::CodegenMinMax(PrimitiveType type, bool min) {
 
   Value* compare = NULL;
   switch (type) {
+    case TYPE_BOOLEAN:
+      if (min) {
+        // For min, return x && y
+        compare = builder.CreateAnd(params[0], params[1]);
+      } else {
+        // For max, return x || y
+        compare = builder.CreateOr(params[0], params[1]);
+      }
+      break;
     case TYPE_TINYINT:
     case TYPE_SMALLINT:
     case TYPE_INT:
@@ -649,14 +658,18 @@ Function* LlvmCodeGen::CodegenMinMax(PrimitiveType type, bool min) {
       DCHECK(false);
   }
 
-  BasicBlock* ret_v1, *ret_v2;
-  CreateIfElseBlocks(fn, "ret_v1", "ret_v2", &ret_v1, &ret_v2);
+  if (type == TYPE_BOOLEAN) {
+    builder.CreateRet(compare);
+  } else {
+    BasicBlock* ret_v1, *ret_v2;
+    CreateIfElseBlocks(fn, "ret_v1", "ret_v2", &ret_v1, &ret_v2);
 
-  builder.CreateCondBr(compare, ret_v1, ret_v2);
-  builder.SetInsertPoint(ret_v1);
-  builder.CreateRet(params[0]);
-  builder.SetInsertPoint(ret_v2);
-  builder.CreateRet(params[1]);
+    builder.CreateCondBr(compare, ret_v1, ret_v2);
+    builder.SetInsertPoint(ret_v1);
+    builder.CreateRet(params[0]);
+    builder.SetInsertPoint(ret_v2);
+    builder.CreateRet(params[1]);
+  }
 
   if (!VerifyFunction(fn)) return NULL;
   return fn;
diff --git a/be/src/exprs/expr.h b/be/src/exprs/expr.h
index 9879d2eed..995893841 100644
--- a/be/src/exprs/expr.h
+++ b/be/src/exprs/expr.h
@@ -177,7 +177,7 @@ struct ExprValue {
   void* SetToMax(PrimitiveType type) {
     switch (type) {
       case TYPE_BOOLEAN:
-        bool_val = false;
+        bool_val = true;
         return &bool_val;
       case TYPE_TINYINT:
         tinyint_val = std::numeric_limits<int8_t>::max();
diff --git a/testdata/workloads/functional-query/queries/QueryTest/aggregation.test b/testdata/workloads/functional-query/queries/QueryTest/aggregation.test
index d95b4886d..6d173fd76 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/aggregation.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/aggregation.test
@@ -101,7 +101,14 @@
 #---- RESULTS
 #'0'
 #====
-# no grouping exprs, cols contain nulls
+# no grouping exprs, cols contain nulls except for bool cols
+select count(bool_col), min(bool_col), max(bool_col)
+from alltypesagg$TABLE
+---- TYPES
+bigint, boolean, boolean
+---- RESULTS
+10000,false,true
+====
 select count(*), count(tinyint_col), min(tinyint_col), max(tinyint_col), sum(tinyint_col),
 avg(tinyint_col)
 from alltypesagg$TABLE
@@ -182,6 +189,13 @@ tinyint, bigint
 NULL,1000
 ====
 # grouping by different data types, with NULLs, grouping expr missing from select list
+select bool_col,min(bool_col),max(bool_col) from alltypesagg$TABLE group by 1
+---- TYPES
+boolean,boolean,boolean
+---- RESULTS
+false,false,false
+true,true,true
+====
 select count(*) from alltypesagg$TABLE group by tinyint_col
 ---- TYPES
 bigint