fix(tianmu): bugfix expression-column base on user value update by ro…

…w order #1903 Cause: Materialize in temptable fill buffer by column order.The user value base on previous column cannot get the true value. Solution: Column base on user value fill buffer by row order.
stoneatom · Jul 5, 2023 · 74afa03 · 74afa03
1 parent d9faa83
commit 74afa03
Show file tree

Hide file tree

Showing 9 changed files with 250 additions and 3 deletions.
diff --git a/mysql-test/suite/tianmu/r/issue1903.result b/mysql-test/suite/tianmu/r/issue1903.result
@@ -0,0 +1,95 @@
+DROP DATABASE IF EXISTS issue1903_test_db;
+CREATE DATABASE issue1903_test_db;
+USE issue1903_test_db;
+CREATE TABLE `c1am_acct_day` (
+`ACCOUNT_ID` decimal(18,0) NOT NULL DEFAULT '-1' COMMENT '账户ID',
+`FISCAL_DATE` date DEFAULT NULL COMMENT '记账日期',
+`BALANCE` decimal(16,2) NOT NULL DEFAULT '0.00' COMMENT '余额',
+`DELETED_FLAG` char(1) NOT NULL DEFAULT '0' COMMENT '记录删除标志 [0]-未删除;[1]-逻辑删除'
+) ENGINE=TIANMU;
+CREATE TABLE `c1md_bank_acct` (
+`ROW_ID` decimal(18,0) NOT NULL DEFAULT '-1' COMMENT 'ROW_ID',
+`CURRENCY_ID` decimal(18,0) NOT NULL DEFAULT '-1' COMMENT '币种ID',
+`DELETED_FLAG` char(1) NOT NULL DEFAULT '0' COMMENT '记录删除标志 [0]-未删除;[1]-逻辑删除'
+) ENGINE=TIANMU;
+INSERT INTO `c1am_acct_day` 
+VALUES 
+(3000000000028804, '2023-04-16', 7628617.08, '0'),
+(3000000000028804, '2023-04-17', 7626656.73, '0'),
+(3000000000028804, '2023-04-18', 7626471.23, '0'),
+(3000000000028806, '2023-04-15', 605253889.19, '0'),
+(3000000000028806, '2023-04-16', 611274357.27, '0'),
+(3000000000028806, '2023-04-17', 605257716.01, '0'),
+(3000000000028808, '2023-04-18', 79322521.29, '0'),
+(3000000000028808, '2023-04-19', 79322521.29, '0'),
+(3000000000028808, '2023-04-20', 79322521.29, '0'),
+(3000000000028809, '2023-04-18', 79322521.29, '0'),
+(3000000000028809, '2023-04-19', 79322521.29, '0'),
+(3000000000028809, '2023-04-20', 79322521.29, '0');
+INSERT INTO `c1md_bank_acct`
+  VALUES
+(3000000000028804, 1, '0'),
+(3000000000028806, 3, '0'),
+(3000000000028808, 15, '0'),
+(3000000000028809, 6, '0');
+SELECT 
+result.* 
+FROM (
+SELECT 
+a.*, 
+@rownum1 := @rownum1 + 1 inde,
+IF(@pxydm1 = a.account_id,@rankno1 := @rankno1 + 1,@rankno1 := 1) AS rankno,
+@pxydm1 := a.account_id 
+FROM (
+SELECT 
+b.CURRENCY_ID,
+a.account_id,
+a.fiscal_date,
+a.balance
+FROM 
+c1am_acct_day a, c1md_bank_acct b
+WHERE a.deleted_flag = '0'
+         		AND b.deleted_flag = '0'
+         		AND a.account_id = b.ROW_ID
+ORDER BY a.account_id, a.fiscal_date) a) result
+WHERE result.rankno = 1;
+CURRENCY_ID	account_id	fiscal_date	balance	inde	rankno	@pxydm1 := a.account_id
+1	3000000000028804	2023-04-16	7628617.08	NULL	1	3000000000028804
+1	3000000000028804	2023-04-17	7626656.73	NULL	1	3000000000028804
+1	3000000000028804	2023-04-18	7626471.23	NULL	1	3000000000028804
+3	3000000000028806	2023-04-15	605253889.19	NULL	1	3000000000028806
+3	3000000000028806	2023-04-16	611274357.27	NULL	1	3000000000028806
+3	3000000000028806	2023-04-17	605257716.01	NULL	1	3000000000028806
+15	3000000000028808	2023-04-18	79322521.29	NULL	1	3000000000028808
+15	3000000000028808	2023-04-19	79322521.29	NULL	1	3000000000028808
+15	3000000000028808	2023-04-20	79322521.29	NULL	1	3000000000028808
+6	3000000000028809	2023-04-18	79322521.29	NULL	1	3000000000028809
+6	3000000000028809	2023-04-19	79322521.29	NULL	1	3000000000028809
+6	3000000000028809	2023-04-20	79322521.29	NULL	1	3000000000028809
+SELECT 
+result.* 
+FROM (
+SELECT 
+a.*, 
+@rownum1 := @rownum1 + 1 inde,
+IF(@pxydm1 = a.account_id,@rankno1 := @rankno1 + 1,@rankno1 := 1) AS rankno,
+@pxydm1 := a.account_id 
+FROM (
+SELECT 
+b.CURRENCY_ID,
+a.account_id,
+a.fiscal_date,
+a.balance
+FROM 
+c1am_acct_day a, c1md_bank_acct b
+WHERE a.deleted_flag = '0'
+         		AND b.deleted_flag = '0'
+         		AND a.account_id = b.ROW_ID
+ORDER BY a.account_id, a.fiscal_date) a) result
+WHERE result.rankno = 1;
+CURRENCY_ID	account_id	fiscal_date	balance	inde	rankno	@pxydm1 := a.account_id
+1	3000000000028804	2023-04-16	7628617.08	NULL	1	3000000000028804
+3	3000000000028806	2023-04-15	605253889.19	NULL	1	3000000000028806
+15	3000000000028808	2023-04-18	79322521.29	NULL	1	3000000000028808
+6	3000000000028809	2023-04-18	79322521.29	NULL	1	3000000000028809
+DROP DATABASE issue1903_test_db;
diff --git a/mysql-test/suite/tianmu/t/issue1903.test b/mysql-test/suite/tianmu/t/issue1903.test
@@ -0,0 +1,88 @@
+--source include/have_tianmu.inc
+
+--disable_warnings
+DROP DATABASE IF EXISTS issue1903_test_db;
+--enable_warnings
+CREATE DATABASE issue1903_test_db;
+USE issue1903_test_db;
+
+CREATE TABLE `c1am_acct_day` (
+  `ACCOUNT_ID` decimal(18,0) NOT NULL DEFAULT '-1' COMMENT '账户ID',
+  `FISCAL_DATE` date DEFAULT NULL COMMENT '记账日期',
+  `BALANCE` decimal(16,2) NOT NULL DEFAULT '0.00' COMMENT '余额',
+  `DELETED_FLAG` char(1) NOT NULL DEFAULT '0' COMMENT '记录删除标志 [0]-未删除;[1]-逻辑删除'
+) ENGINE=TIANMU;
+
+CREATE TABLE `c1md_bank_acct` (
+  `ROW_ID` decimal(18,0) NOT NULL DEFAULT '-1' COMMENT 'ROW_ID',
+  `CURRENCY_ID` decimal(18,0) NOT NULL DEFAULT '-1' COMMENT '币种ID',
+  `DELETED_FLAG` char(1) NOT NULL DEFAULT '0' COMMENT '记录删除标志 [0]-未删除;[1]-逻辑删除'
+) ENGINE=TIANMU;
+
+INSERT INTO `c1am_acct_day` 
+  VALUES 
+    (3000000000028804, '2023-04-16', 7628617.08, '0'),
+    (3000000000028804, '2023-04-17', 7626656.73, '0'),
+    (3000000000028804, '2023-04-18', 7626471.23, '0'),
+    (3000000000028806, '2023-04-15', 605253889.19, '0'),
+    (3000000000028806, '2023-04-16', 611274357.27, '0'),
+    (3000000000028806, '2023-04-17', 605257716.01, '0'),
+    (3000000000028808, '2023-04-18', 79322521.29, '0'),
+    (3000000000028808, '2023-04-19', 79322521.29, '0'),
+    (3000000000028808, '2023-04-20', 79322521.29, '0'),
+    (3000000000028809, '2023-04-18', 79322521.29, '0'),
+    (3000000000028809, '2023-04-19', 79322521.29, '0'),
+    (3000000000028809, '2023-04-20', 79322521.29, '0');
+
+INSERT INTO `c1md_bank_acct`
+  VALUES
+    (3000000000028804, 1, '0'),
+    (3000000000028806, 3, '0'),
+    (3000000000028808, 15, '0'),
+    (3000000000028809, 6, '0');
+
+SELECT 
+ 	result.* 
+ FROM (
+     SELECT 
+     	a.*, 
+     	@rownum1 := @rownum1 + 1 inde,
+     	IF(@pxydm1 = a.account_id,@rankno1 := @rankno1 + 1,@rankno1 := 1) AS rankno,
+     	@pxydm1 := a.account_id 
+     FROM (
+         SELECT 
+         	b.CURRENCY_ID,
+         	a.account_id,
+         	a.fiscal_date,
+         	a.balance
+         FROM 
+         	c1am_acct_day a, c1md_bank_acct b
+         WHERE a.deleted_flag = '0'
+         		AND b.deleted_flag = '0'
+         		AND a.account_id = b.ROW_ID
+         ORDER BY a.account_id, a.fiscal_date) a) result
+WHERE result.rankno = 1;
+
+SELECT 
+ 	result.* 
+ FROM (
+     SELECT 
+     	a.*, 
+     	@rownum1 := @rownum1 + 1 inde,
+     	IF(@pxydm1 = a.account_id,@rankno1 := @rankno1 + 1,@rankno1 := 1) AS rankno,
+     	@pxydm1 := a.account_id 
+     FROM (
+         SELECT 
+         	b.CURRENCY_ID,
+         	a.account_id,
+         	a.fiscal_date,
+         	a.balance
+         FROM 
+         	c1am_acct_day a, c1md_bank_acct b
+         WHERE a.deleted_flag = '0'
+         		AND b.deleted_flag = '0'
+         		AND a.account_id = b.ROW_ID
+         ORDER BY a.account_id, a.fiscal_date) a) result
+WHERE result.rankno = 1;
+
+DROP DATABASE issue1903_test_db;
diff --git a/storage/tianmu/core/mysql_expression.cpp b/storage/tianmu/core/mysql_expression.cpp
@@ -352,6 +352,31 @@ Item *MysqlExpression::TransformTree(Item *root, TransformDirection dir) {
 
 MysqlExpression::SetOfVars &MysqlExpression::GetVars() { return vars; }
 
+bool item_func_suser_traverse(Item_func *it_f) {
+  if (it_f->functype() == Item_func::Functype::SUSERVAR_FUNC)
+    return true;
+  bool res = false;
+  Item **args = nullptr;
+  uint cnt = it_f->arg_count;
+  if (it_f->arg_count > 0)
+    args = it_f->arguments();
+  for (uint i = 0; i < cnt && !res; ++i) {
+    if (args[i]->type() == Item::Type::FUNC_ITEM)
+      res = item_func_suser_traverse(reinterpret_cast<Item_func *>(args[i]));
+  }
+  return res;
+}
+
+/* Only return true when item set user value.
+ * It is equal to one of node in item tree is Item_func_set_user_var.
+ */
+bool MysqlExpression::BaseOnUserValue() {
+  if (item->type() != Item::Type::FUNC_ITEM)
+    return false;
+  Item_func *item_f = reinterpret_cast<Item_func *>(item);
+  return item_func_suser_traverse(item_f);
+}
+
 void MysqlExpression::SetBufsOrParams(var_buf_t *bufs) {
   DEBUG_ASSERT(bufs);
   for (auto &it : tianmu_fields_cache) {

diff --git a/storage/tianmu/core/mysql_expression.h b/storage/tianmu/core/mysql_expression.h
@@ -61,6 +61,7 @@ class MysqlExpression {
   Item *GetItem() { return item; }
   const tianmu_fields_cache_t &GetTIANMUItems() { return tianmu_fields_cache; }
   bool IsDeterministic() { return deterministic; }
+  bool BaseOnUserValue();
   /*! \brief Tests if other MysqlExpression is same as this one.
    *
    * \param other - equality to this MysqlExpression is being questioned.

diff --git a/storage/tianmu/core/temp_table.h b/storage/tianmu/core/temp_table.h
@@ -88,6 +88,7 @@ class TempTable : public JustATable {
              !term.vc->IsConst();  // constant value, the buffer is already
                                    // filled in
     }
+    bool BaseUserVar() { return term.vc->BaseUserVar(); }
 
     enum phys_col_t ColType() const override { return phys_col_t::ATTR; }
     //! Use in cases where actual string length is less than declared, before
@@ -387,6 +388,7 @@ class TempTable : public JustATable {
   void MoveVC(vcolumn::VirtualColumn *vc, std::vector<vcolumn::VirtualColumn *> &from,
               std::vector<vcolumn::VirtualColumn *> &to);
   void FillbufferTask(Attr *attr, Transaction *txn, MIIterator *page_start, int64_t start_row, int64_t page_end);
+  void FillBufferByRow(std::vector<Attr *> attrs, MIIterator *page_start, int64_t start_row, int64_t page_end);
   size_t TaskPutValueInST(MIIterator *it, Transaction *ci, SorterWrapper *st);
   bool HasTempTable() const { return has_temp_table; }
 

diff --git a/storage/tianmu/core/temp_table_low.cpp b/storage/tianmu/core/temp_table_low.cpp
@@ -356,6 +356,7 @@ void TempTable::FillMaterializedBuffers(int64_t local_limit, int64_t local_offse
   int64_t row = local_offset;
   std::vector<char> skip_parafilloutput;
   std::set<int> set_vcid;
+  std::vector<Attr *> attrs_fillbyrow;
 
   for (auto &attr : attrs) {
     // check if there is duplicated columns, mark skip flag for yes
@@ -367,6 +368,8 @@ void TempTable::FillMaterializedBuffers(int64_t local_limit, int64_t local_offse
         skip = 1;
       } else {
         set_vcid.insert(attr->term.vc_id);
+        if (attr->BaseUserVar())
+          skip = 1;
       }
     }
     skip_parafilloutput.push_back(skip);
@@ -418,9 +421,19 @@ void TempTable::FillMaterializedBuffers(int64_t local_limit, int64_t local_offse
     }
     res.get_all_with_except();
 
-    for (uint i = 1; i < attrs.size(); i++)
-      if (skip_parafilloutput[i])
+    for (uint i = 1; i < attrs.size(); i++) {
+      if (attrs[i]->BaseUserVar()) {
+        if (attrs[i]->NeedFill()) {
+          attrs_fillbyrow.push_back(attrs[i]);
+        }
+      } else if (skip_parafilloutput[i]) {
         FillbufferTask(attrs[i], current_txn_, &page_start, start_row, page_end);
+      }
+    }
+    // if column base on user value, fill buffer by row order
+    if (!attrs_fillbyrow.empty()) {
+      FillBufferByRow(attrs_fillbyrow, &page_start, start_row, page_end);
+    }
 
     if (lazy)
       break;
@@ -573,6 +586,22 @@ void TempTable::FillbufferTask(Attr *attr, Transaction *txn, MIIterator *page_st
   }
 }
 
+void TempTable::FillBufferByRow(std::vector<Attr *> attrs, MIIterator *page_start, int64_t start_row,
+                                int64_t page_end) {
+  MIIterator itr(*page_start);
+  int64_t n, cnt = page_end - start_row;
+  for (size_t n = 0; n < cnt; ++n) {
+    for (auto attr : attrs) {
+      if (itr.PackrowStarted() || n == 0) {
+        attr->term.vc->LockSourcePacks(itr);
+      }
+      attr->FillValue(itr, start_row + n);
+    }
+    ++itr;
+  }
+  for (auto attr : attrs) attr->term.vc->UnlockSourcePacks();
+}
+
 size_t TempTable::TaskPutValueInST(MIIterator *it, Transaction *ci, SorterWrapper *st) {
   size_t local_row = 0;
   bool continue_now = true;

diff --git a/storage/tianmu/vc/expr_column.cpp b/storage/tianmu/vc/expr_column.cpp
@@ -30,7 +30,7 @@ ExpressionColumn::ExpressionColumn(core::MysqlExpression *expr, core::TempTable
       deterministic_(expr ? expr->IsDeterministic() : true) {
   const std::vector<core::JustATable *> *tables = &temp_table->GetTables();
   const std::vector<int> *aliases = &temp_table->GetAliases();
-
+  use_usr_var_ = false;
   if (expr_) {
     vars_ = expr_->GetVars();  // get all variables from complex term
     first_eval_ = true;
@@ -77,6 +77,9 @@ ExpressionColumn::ExpressionColumn(core::MysqlExpression *expr, core::TempTable
 
     // if (status == VC_EXPR && var_map_.size() == 0 )
     //	status = VC_CONST;
+
+    // if expr calculate base on user value set the value use_usr_val_ true
+    use_usr_var_ = expr_->BaseOnUserValue();
   } else {
     DEBUG_ASSERT(!"unexpected!!");
   }
@@ -90,6 +93,7 @@ ExpressionColumn::ExpressionColumn(const ExpressionColumn &ec)
       var_buf_(ec.var_buf_),
       deterministic_(ec.deterministic_) {
   var_map_ = ec.var_map_;
+  use_usr_var_ = ec.use_usr_var_;
 }
 
 void ExpressionColumn::SetParamTypes(core::MysqlExpression::TypOfVars *types) { expr_->EvalType(types); }

diff --git a/storage/tianmu/vc/expr_column.h b/storage/tianmu/vc/expr_column.h
@@ -118,6 +118,7 @@ class ExpressionColumn : public VirtualColumn {
   }
   core::MysqlExpression *expr_;  //!= nullptr if ExpressionColumn encapsulates an expression. Note - a
                                  //! constant is an expression
+  bool BaseUserVar() { return use_usr_var_; }
 
  private:
   /*! \brief Set variable buffers with values for a given iterator.
@@ -137,6 +138,7 @@ class ExpressionColumn : public VirtualColumn {
   //! value for a given row is always the same or not? e.g. currenttime() is not
   //! deterministic
   bool deterministic_;
+  bool use_usr_var_;
 };
 }  // namespace vcolumn
 }  // namespace Tianmu

diff --git a/storage/tianmu/vc/virtual_column.h b/storage/tianmu/vc/virtual_column.h
@@ -44,6 +44,7 @@ class VirtualColumn : public VirtualColumnBase {
 
   void LockSourcePacks(const core::MIIterator &mit) override { vc_pack_guard_.LockPackrow(mit); }
   void UnlockSourcePacks() override { vc_pack_guard_.UnlockAll(); }
+  virtual bool BaseUserVar() { return false; }
 
  private:
   core::VCPackGuardian vc_pack_guard_;