incubator-quickstep git commit: Fix the bug with min/max aggregation

[jianqiao]

Repository: incubator-quickstep
Updated Branches:
  refs/heads/fix-compare-aggregate [created] 5616a646b


Fix the bug with min/max aggregation


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

Branch: refs/heads/fix-compare-aggregate
Commit: 5616a646bba0aa189bb7179f260d21431a8d1aa6
Parents: 302f2cb
Author: Jianqiao Zhu <jianq...@cs.wisc.edu>
Authored: Wed Aug 2 15:47:30 2017 -0500
Committer: Jianqiao Zhu <jianq...@cs.wisc.edu>
Committed: Wed Aug 2 15:47:30 2017 -0500

----------------------------------------------------------------------
 .../comparisons/LiteralComparators-inl.hpp      | 72 ++++++++++++++++----
 1 file changed, 57 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5616a646/types/operations/comparisons/LiteralComparators-inl.hpp
----------------------------------------------------------------------
diff --git a/types/operations/comparisons/LiteralComparators-inl.hpp 
b/types/operations/comparisons/LiteralComparators-inl.hpp
index fd59e2e..b7271ad 100644
--- a/types/operations/comparisons/LiteralComparators-inl.hpp
+++ b/types/operations/comparisons/LiteralComparators-inl.hpp
@@ -543,7 +543,11 @@ TypedValue LiteralUncheckedComparator<ComparisonFunctor,
         const TypedValue &current,
         ValueAccessor *accessor,
         const attribute_id value_accessor_id) const {
-  const void *current_literal = current.isNull() ? nullptr : 
current.getDataPtr();
+  bool is_null = current.isNull();
+  LeftCppType current_literal;
+  if (!is_null) {
+    current_literal = current.getLiteral<LeftCppType>();
+  }
 
   InvokeOnValueAccessorMaybeTupleIdSequenceAdapter(
       accessor,
@@ -555,30 +559,68 @@ TypedValue LiteralUncheckedComparator<ComparisonFunctor,
       std::unique_ptr<const ColumnAccessor<left_nullable>>
           column_accessor(accessor->template 
getColumnAccessor<left_nullable>(value_accessor_id));
       DCHECK(column_accessor != nullptr);
-      while (accessor->next()) {
-        const void *va_value = column_accessor->getUntypedValue();
-        if (left_nullable && !va_value) {
-          continue;
+
+      // Locate the first non-null value.
+      if (is_null) {
+        const void *va_value = nullptr;
+        while (accessor->next()) {
+          va_value = column_accessor->getUntypedValue();
+          if (!left_nullable || va_value) {
+            break;
+          }
+        }
+        if (va_value != nullptr) {
+          is_null = false;
+          current_literal = *static_cast<const LeftCppType*>(va_value);
         }
-        if (!current_literal || this->compareDataPtrsHelper<true>(va_value, 
current_literal)) {
-          current_literal = va_value;
+      }
+
+      // Aggregate on the remaining values.
+      if (!accessor->iterationFinished()) {
+        while (accessor->next()) {
+          const void *va_value = column_accessor->getUntypedValue();
+          if (left_nullable && !va_value) {
+            continue;
+          }
+          if (this->compareDataPtrsHelper<true>(va_value, &current_literal)) {
+            current_literal = *static_cast<const LeftCppType*>(va_value);
+          }
         }
       }
     } else {
-      while (accessor->next()) {
-        const void *va_value = accessor->template 
getUntypedValue<left_nullable>(value_accessor_id);
-        if (left_nullable && !va_value) {
-          continue;
+      // Locate the first non-null value.
+      if (is_null) {
+        const void *va_value = nullptr;
+        while (accessor->next()) {
+          va_value = accessor->template 
getUntypedValue<left_nullable>(value_accessor_id);
+          if (!left_nullable || va_value) {
+            break;
+          }
         }
-        if (!current_literal || this->compareDataPtrsHelper<true>(va_value, 
current_literal)) {
-          current_literal = va_value;
+        if (va_value != nullptr) {
+          is_null = false;
+          current_literal = *static_cast<const LeftCppType*>(va_value);
+        }
+      }
+
+      // Aggregate on the remaining values.
+      if (!accessor->iterationFinished()) {
+        while (accessor->next()) {
+          const void *va_value =
+              accessor->template 
getUntypedValue<left_nullable>(value_accessor_id);
+          if (left_nullable && !va_value) {
+            continue;
+          }
+          if (this->compareDataPtrsHelper<true>(va_value, &current_literal)) {
+            current_literal = *static_cast<const LeftCppType*>(va_value);
+          }
         }
       }
     }
   });
 
-  if (current_literal) {
-    return TypedValue(*static_cast<const LeftCppType*>(current_literal));
+  if (!is_null) {
+    return TypedValue(current_literal);
   } else {
     return TypedValue(current.getTypeID());
   }