maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

Next patch, MDEV 7701

+++ ./003_mariadb-10.2.0-vp/sql/ha_partition.cc    2016-05-05
21:25:04.289731712 +0900
@@ -7238,13 +7239,30 @@ int ha_partition::extra(enum ha_extra_fu
   }
     /* Category 9) Operations only used by MERGE */
   case HA_EXTRA_ADD_CHILDREN_LIST:
+    DBUG_RETURN(loop_extra(operation));
   case HA_EXTRA_ATTACH_CHILDREN:
+    int result;
+    handler **file;
+    if ((result = loop_extra(operation)))
+      DBUG_RETURN(result);
+    m_num_locks = 0;
+    additional_table_flags =
+      (HA_CAN_BG_SEARCH | HA_CAN_BG_INSERT | HA_CAN_BG_UPDATE);
+    file = m_file;
+    do
+    {
+      m_num_locks += (*file)->lock_count();
+      additional_table_flags &= ~((ulonglong) ((*file)->ha_table_flags() ^
+        (HA_CAN_BG_SEARCH | HA_CAN_BG_INSERT | HA_CAN_BG_UPDATE)));

The above test could be simple:

additional_table_flags &= (ulonglong) ((*file)->ha_table_flags();

Should be the same thing.  I did however remove this code, as
HA_CAN_BG... are not used anywhere in the current spider code. See
comment below.

I also changed so that m_num_locks always contains the upper limit
number of locks needed.  Without this change, the call to lock_count() would
have returneed m_total_parts * 'new-counted-locks' which would be been
way too much.


@@ -9123,6 +9141,19 @@ const COND *ha_partition::cond_push(cons
   handler **file= m_file;
   COND *res_cond = NULL;
   DBUG_ENTER("ha_partition::cond_push");
+#ifdef HANDLER_HAS_TOP_TABLE_FIELDS
+  if (set_top_table_fields)
+  {
+    do
+    {
+      if ((*file)->set_top_table_and_fields(top_table,
+                                        top_table_field,
+                                        top_table_fields))
+        DBUG_RETURN(cond);
+    } while (*(++file));
+    file= m_file;
+  }
+#endif

I added this without the define, but I merged it to the loop for cond_push.

Instead of having defines in the sql code, I will fix that in spider and vp
that I will set the proper defines based on the MariaDB version.

However, I would like to have a clear comment the purpose of the function
set_top_table_and_fields().

 struct st_mysql_storage_engine partition_storage_engine=
 { MYSQL_HANDLERTON_INTERFACE_VERSION };

+++ ./003_mariadb-10.2.0-vp/sql/handler.h    2016-05-05 21:25:04.314731713 +0900
@@ -255,6 +257,11 @@ enum enum_alter_inplace_result {
  */
 #define HA_BINLOG_FLAGS (HA_BINLOG_ROW_CAPABLE | HA_BINLOG_STMT_CAPABLE)

+#define HA_CAN_BG_SEARCH       (1LL << 45)
+#define HA_CAN_BG_INSERT       (1LL << 46)
+#define HA_CAN_BG_UPDATE       (1LL << 47)
+#define HA_CAN_MULTISTEP_MERGE (1LL << 48)

Do you really need HA_CAN_BG_XXX flags?
I checked your spider tree and this is not used anywhere, so I think
we should leave them out for now.
If we don't need these, we can remove the loop for setting them in
HA_EXTRA_ATTACH_CHILDREN too.

If we need them, I would like to have a comment for why they are needed and
when we will need then add these in a separate patch.

@@ -3525,6 +3540,29 @@ public:
    Pops the top if condition stack, if stack is not empty.
  */
  virtual void cond_pop() { return; };
+ virtual int set_top_table_and_fields(TABLE *top_table,
+                                      Field **top_table_field,
+                                      uint top_table_fields)
+ {
+   if (!set_top_table_fields)
+   {
+     set_top_table_fields = TRUE;
+     this->top_table = top_table;
+     this->top_table_field = top_table_field;
+     this->top_table_fields = top_table_fields;
+   }
+   return 0;
+ }

Why is this int and not void ?
Do you have in mind that this may be possible to fail in the future?
If it's never going to change, then better to do this void.

I will make it void for now and change the relevant code. We can make it
int again if needed in the future.

diff -Narup ./002_mariadb-10.2.0-spider/sql/sql_base.cc
./003_mariadb-10.2.0-vp/sql/sql_base.cc
--- ./002_mariadb-10.2.0-spider/sql/sql_base.cc    2016-04-17
02:47:27.000000000 +0900
+++ ./003_mariadb-10.2.0-vp/sql/sql_base.cc    2016-05-05
21:25:04.319731713 +0900
@@ -1475,6 +1475,10 @@ unique_table(THD *thd, TABLE_LIST *table

   table= table->find_table_for_update();

-  if (table->table && table->table->file->ht->db_type == DB_TYPE_MRG_MYISAM)
-  {
-    TABLE_LIST *child;
+  if (table->table &&
+    (
+      table->table->file->ht->db_type == DB_TYPE_MRG_MYISAM ||
+      (table->table->file->ha_table_flags() & HA_CAN_MULTISTEP_MERGE)
+    )
+  )
+  continue

I added HA_CAN_MULTISTEP_MERGE as a flag to DB_TYPE_MRG_MYISAM, which
allwoed me to make the above tests and the following identical ones
much
simpler.

This is now pushed into 10.2-spider

Regards,
Monty