From f5a8d228a00a6eaf77ad61a0d2915d85e2cbb989 Mon Sep 17 00:00:00 2001
From: Gagan Goel <gagan.nith@gmail.com>
Date: Wed, 1 Jul 2020 17:39:22 -0400
Subject: [PATCH] Port of commit ba731bdc6a80e88d32e7440044b548c3e3edc591 from
 server/columnstore_cache

Commit message:

Fixed crashed bug on simple insert

Other things:
- Added test from columnstore team
- Fixed two reported bugs from columnstore team
- Call free_locks as part of start_trans() instead of get_status()
  to ensure that we have locks both for cached table and cache table
  before we try to free any.
- Store pointers to lock->get_status and lock->update_status for the
  cached table. Was needed by ha_tina in flush_insert_cache to make
  new insert rows visible for the SELECT that caused the flush
---
 dbcon/mysql/ha_mcs.cpp | 40 +++++++++++++++++++++-------------------
 dbcon/mysql/ha_mcs.h   |  1 +
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/dbcon/mysql/ha_mcs.cpp b/dbcon/mysql/ha_mcs.cpp
index ffe91d0a5..5c419e814 100644
--- a/dbcon/mysql/ha_mcs.cpp
+++ b/dbcon/mysql/ha_mcs.cpp
@@ -1226,6 +1226,7 @@ bool ha_mcs::is_crashed() const
 my_bool get_status_and_flush_cache(void *param,
                                    my_bool concurrent_insert);
 
+
 /*
   Create a name for the cache table
 */
@@ -1241,7 +1242,9 @@ static void create_cache_name(char *to, const char *name)
  THR_LOCK wrapper functions
 
   The idea of these is to highjack 'THR_LOCK->get_status() so that if this
-  is called in a non-insert context then we will flush the cache
+  is called in a non-insert context then we will flush the cache.
+  We also hijack THR_LOCK->start_trans() to free any locks on the cache
+  if the command was not an insert command.
 *****************************************************************************/
 
 /*
@@ -1263,7 +1266,7 @@ my_bool get_status_and_flush_cache(void *param,
     All Aria get_status functions takes Maria handler as the parameter
   */
   if (cache->share->org_lock.get_status)
-    (*cache->share->org_lock.get_status)(&cache->cache_handler->file,
+    (*cache->share->org_lock.get_status)(cache->cache_handler->file,
                                          concurrent_insert);
 
   /* If first get_status() call for this table, flush cache if needed */
@@ -1281,20 +1284,28 @@ my_bool get_status_and_flush_cache(void *param,
     }
   }
 
-  if (!cache->insert_command)
-    cache->free_locks();
-
   return (0);
 }
 
-/* Pass through functions for all the THR_LOCK virtual functions */
+/*
+  start_trans() is called when all locks has been given
+  If this was not an insert command then we can free the write lock on
+  the cache table and also downgrade external lock for the cached table
+  to F_READ
+*/
 
-static my_bool cache_start_trans(void* param)
+my_bool cache_start_trans(void* param)
 {
   ha_mcs_cache *cache= (ha_mcs_cache*) param;
+
+  if (!cache->insert_command)
+    cache->free_locks();
+
   return (*cache->share->org_lock.start_trans)(cache->cache_handler->file);
 }
 
+/* Pass through functions for all the THR_LOCK virtual functions */
+
 static void cache_copy_status(void* to, void *from)
 {
   ha_mcs_cache *to_cache= (ha_mcs_cache*) to, *from_cache= (ha_mcs_cache*) from;
@@ -1345,6 +1356,7 @@ ha_mcs_cache_share *find_cache_share(const char *name)
   {
     if (!strcmp(pos->name, name))
     {
+      pos->open_count++;
       mysql_mutex_unlock(&LOCK_cache_share);
       return(pos);
     }
@@ -1522,7 +1534,7 @@ uint ha_mcs_cache::lock_count(void) const
 {
   /*
     If we are doing an insert or if we want to flush the cache, we have to lock
-    both MyISAM table and normal table.
+    both the Aria table and normal table.
   */
   return 2;
 }
@@ -1886,8 +1898,6 @@ bool ha_mcs_cache::rows_cached()
 void ha_mcs_cache::free_locks()
 {
   /* We don't need to lock cache_handler anymore as it's already flushed */
-
-  mysql_mutex_unlock(&cache_handler->file->lock.lock->mutex);
   thr_unlock(&cache_handler->file->lock, 0);
 
   /* Restart transaction for columnstore table */
@@ -1896,11 +1906,9 @@ void ha_mcs_cache::free_locks()
     parent::external_lock(table->in_use, F_UNLCK);
     parent::external_lock(table->in_use, original_lock_type);
   }
-
-  /* Needed as we are going back to end of thr_lock() */
-  mysql_mutex_lock(&cache_handler->file->lock.lock->mutex);
 }
 
+
 /**
    Copy data from cache to ColumnStore
 
@@ -1950,12 +1958,6 @@ end:
       to use it.
     */
     from->delete_all_rows();
-
-    /*
-      This was not an insert command, so we can delete the thr lock
-      (We are not going to use the insert cache for this statement anymore)
-    */
-    free_locks();
   }
   DBUG_RETURN(error);
 }
diff --git a/dbcon/mysql/ha_mcs.h b/dbcon/mysql/ha_mcs.h
index 6b51c201a..c3751fa47 100644
--- a/dbcon/mysql/ha_mcs.h
+++ b/dbcon/mysql/ha_mcs.h
@@ -304,6 +304,7 @@ public:
   int flush_insert_cache();
   friend my_bool get_status_and_flush_cache(void *param,
                                             my_bool concurrent_insert);
+  friend my_bool cache_start_trans(void *param);
 };
 
 #endif //HA_MCS_H__