From 15f8652a7233b70b3c8266f8ab9dd1191e854f96 Mon Sep 17 00:00:00 2001
From: obdev <obdev@oceanbase.com>
Date: Mon, 6 Feb 2023 14:18:20 +0800
Subject: [PATCH] fix sort core at aqs item's len overflow

---
 src/sql/engine/sort/ob_sort_op.cpp      |  6 ++--
 src/sql/engine/sort/ob_sort_op_impl.cpp | 22 +++++++++----
 src/sql/engine/sort/ob_sort_op_impl.h   | 44 ++++++++++++-------------
 3 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/src/sql/engine/sort/ob_sort_op.cpp b/src/sql/engine/sort/ob_sort_op.cpp
index 9fc10217b5..579c508984 100644
--- a/src/sql/engine/sort/ob_sort_op.cpp
+++ b/src/sql/engine/sort/ob_sort_op.cpp
@@ -411,8 +411,9 @@ int ObSortOp::init_prefix_sort(int64_t tenant_id,
   } else {
     read_func_ = &ObSortOp::prefix_sort_impl_next;
   }
+  int aqs_head = MY_SPEC.enable_encode_sortkey_opt_ ? sizeof(oceanbase::sql::ObSortOpImpl::AQSItem) : 0;
   prefix_sort_impl_.set_input_rows(row_count);
-  prefix_sort_impl_.set_input_width(MY_SPEC.width_);
+  prefix_sort_impl_.set_input_width(MY_SPEC.width_ + aqs_head);
   prefix_sort_impl_.set_operator_type(MY_SPEC.type_);
   prefix_sort_impl_.set_operator_id(MY_SPEC.id_);
   prefix_sort_impl_.set_io_event_observer(&io_event_observer_);
@@ -433,8 +434,9 @@ int ObSortOp::init_sort(int64_t tenant_id,
   } else {
     read_func_ = &ObSortOp::sort_impl_next;
   }
+  int aqs_head = MY_SPEC.enable_encode_sortkey_opt_ ? sizeof(oceanbase::sql::ObSortOpImpl::AQSItem) : 0;
   sort_impl_.set_input_rows(row_count);
-  sort_impl_.set_input_width(MY_SPEC.width_);
+  sort_impl_.set_input_width(MY_SPEC.width_ + aqs_head);
   sort_impl_.set_operator_type(MY_SPEC.type_);
   sort_impl_.set_operator_id(MY_SPEC.id_);
   sort_impl_.set_io_event_observer(&io_event_observer_);
diff --git a/src/sql/engine/sort/ob_sort_op_impl.cpp b/src/sql/engine/sort/ob_sort_op_impl.cpp
index 4682c10a61..225688a1e5 100644
--- a/src/sql/engine/sort/ob_sort_op_impl.cpp
+++ b/src/sql/engine/sort/ob_sort_op_impl.cpp
@@ -26,10 +26,15 @@ namespace sql
 
 /************************************* start ObSortOpImpl *********************************/
 ObSortOpImpl::ObAdaptiveQS::ObAdaptiveQS(common::ObIArray<ObChunkDatumStore::StoredRow *> &sort_rows,
-                                         common::ObIAllocator &alloc, int64_t rows_begin,
-                                         int64_t rows_end, bool &can_encode)
+                                         common::ObIAllocator &alloc)
   : orig_sort_rows_(sort_rows),
     alloc_(alloc)
+{
+}
+
+int ObSortOpImpl::ObAdaptiveQS::init(common::ObIArray<ObChunkDatumStore::StoredRow *> &sort_rows,
+                                         common::ObIAllocator &alloc, int64_t rows_begin,
+                                         int64_t rows_end, bool &can_encode)
 {
   int ret = OB_SUCCESS;
   can_encode = true;
@@ -56,6 +61,7 @@ ObSortOpImpl::ObAdaptiveQS::ObAdaptiveQS(common::ObIArray<ObChunkDatumStore::Sto
       if (item.len_>1) item.sub_cache_[1] = item.key_ptr_[1];
     }
   }
+  return ret;
 }
 
 /*
@@ -1352,8 +1358,10 @@ int ObSortOpImpl::do_partition_sort(common::ObIArray<ObChunkDatumStore::StoredRo
       if (comp_.cmp_start_ != comp_.cmp_end_) {
         if (enable_encode_sortkey_) {
           bool can_encode = true;
-          ObAdaptiveQS aqs(rows, allocator, rows_last, rows_idx, can_encode);
-          if (can_encode) {
+          ObAdaptiveQS aqs(rows, allocator);
+          if (OB_FAIL(aqs.init(rows, allocator, rows_last, rows_idx, can_encode))) {
+            LOG_WARN("failed to init aqs", K(ret));
+          } else if (can_encode) {
             aqs.sort(rows_last, rows_idx);
           } else {
             enable_encode_sortkey_ = false;
@@ -1631,8 +1639,10 @@ int ObSortOpImpl::sort_inmem_data()
         do_partition_sort(*rows_, begin, rows_->count());
       } else if (enable_encode_sortkey_) {
         bool can_encode = true;
-        ObAdaptiveQS aqs(*rows_, mem_context_->get_malloc_allocator(), begin, rows_->count(), can_encode);
-        if (can_encode) {
+        ObAdaptiveQS aqs(*rows_, mem_context_->get_malloc_allocator());
+        if (OB_FAIL(aqs.init(*rows_, mem_context_->get_malloc_allocator(), begin, rows_->count(), can_encode))) {
+          LOG_WARN("failed to init aqs", K(ret));
+        } else if (can_encode) {
           aqs.sort(begin, rows_->count());
         } else {
           enable_encode_sortkey_ = false;
diff --git a/src/sql/engine/sort/ob_sort_op_impl.h b/src/sql/engine/sort/ob_sort_op_impl.h
index f3bce71bc3..51d7196943 100644
--- a/src/sql/engine/sort/ob_sort_op_impl.h
+++ b/src/sql/engine/sort/ob_sort_op_impl.h
@@ -335,37 +335,23 @@ public:
     Compare &compare_;
   };
 
-protected:
-  class MemEntifyFreeGuard
-  {
-  public:
-    explicit MemEntifyFreeGuard(lib::MemoryContext &entify) : entify_(entify) {}
-    ~MemEntifyFreeGuard()
-    {
-      if (NULL != entify_) {
-        DESTROY_CONTEXT(entify_);
-        entify_ = NULL;
-      }
-    }
-    lib::MemoryContext &entify_;
-  };
-
   struct AQSItem {
-    short len_;
-    unsigned char sub_cache_[2];
     unsigned char *key_ptr_;
     ObChunkDatumStore::StoredRow *row_ptr_;
-    AQSItem() : len_(0),
-                sub_cache_(),
-                key_ptr_(NULL),
-                row_ptr_(NULL)
+    uint32_t len_;
+    unsigned char sub_cache_[2];
+    AQSItem() : key_ptr_(NULL),
+                row_ptr_(NULL),
+                len_(0),
+                sub_cache_()
     {
     }
     TO_STRING_KV(K_(len), K_(sub_cache), K_(key_ptr), KP(row_ptr_));
   };
   class ObAdaptiveQS {
     public:
-      ObAdaptiveQS(common::ObIArray<ObChunkDatumStore::StoredRow *> &sort_rows,
+      ObAdaptiveQS(common::ObIArray<ObChunkDatumStore::StoredRow *> &sort_rows, common::ObIAllocator &alloc);
+      int init(common::ObIArray<ObChunkDatumStore::StoredRow *> &sort_rows,
                    common::ObIAllocator &alloc, int64_t rows_begin, int64_t rows_end, bool &can_encode);
       ~ObAdaptiveQS() {
         reset();
@@ -408,6 +394,20 @@ protected:
       common::ObIAllocator &alloc_;
   };
 
+protected:
+  class MemEntifyFreeGuard
+  {
+  public:
+    explicit MemEntifyFreeGuard(lib::MemoryContext &entify) : entify_(entify) {}
+    ~MemEntifyFreeGuard()
+    {
+      if (NULL != entify_) {
+        DESTROY_CONTEXT(entify_);
+        entify_ = NULL;
+      }
+    }
+    lib::MemoryContext &entify_;
+  };
   //Optimize mem usage/performance of top-n sort:
   //https://aone.alibaba-inc.com/project/81079/issue/8572633
   //Record buf_len of each allocated row. When old row pop-ed out of the heap
-- 
GitLab