maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Oleksandr!

On Mar 15, Oleksandr Byelkin wrote:
> >> diff --git a/include/mysql_com.h b/include/mysql_com.h
> >> index c399520022d..9fac5edd1bc 100644
> >> --- a/include/mysql_com.h
> >> +++ b/include/mysql_com.h
> >> @@ -551,7 +551,8 @@ enum enum_cursor_type
> >>     CURSOR_TYPE_NO_CURSOR= 0,
> >>     CURSOR_TYPE_READ_ONLY= 1,
> >>     CURSOR_TYPE_FOR_UPDATE= 2,
> >> -  CURSOR_TYPE_SCROLLABLE= 4
> >> +  CURSOR_TYPE_SCROLLABLE= 4,
> >> +  INSERT_ID_REQUEST= 128
> > That's very weird. Why would "INSERT_ID_REQUEST" be a flag in the
> > "cursor_type"?
> >
> > Don't put this flag into enum_cursor_type
> It is flags and it happened that they are about cursors, but field of 
> flags can be used for other flags also (IMHO)

Yes. I saw that it only flags. I said just "don't put this flag into
enum_cursor_type". That is, you can store it in the same flag variable
together with cursor flags. But don't put it in the same enum.

Either

 #define INSERT_ID_REQUEST 128

or a separate enum with non-cursor flags.

> >>   };
> >>   
> >>   
> >> diff --git a/sql/sql_class.cc b/sql/sql_class.cc
> >> index 8fabc8f593e..53591c371c0 100644
> >> --- a/sql/sql_class.cc
> >> +++ b/sql/sql_class.cc
> >> @@ -7365,4 +7366,119 @@ bool Discrete_intervals_list::append(Discrete_interval *new_interval)
> >>     DBUG_RETURN(0);
> >>   }
> >>   
> >> +bool THD::init_collecting_insert_id()
> >> +{
> >> +  if (!insert_ids)
> >> +  {
> >> +    void *buff;
> >> +    if (!(my_multi_malloc(MYF(MY_WME), &insert_ids, sizeof(DYNAMIC_ARRAY),
> >> +                          &buff, sizeof(insert_id_desc) * 10,
> >> +                          NullS)) ||
> >> +        my_init_dynamic_array2(insert_ids, sizeof(insert_id_desc),
> >> +                               buff, 10, 100, MYF(MY_WME)))
> > Huh?
> > 1. You allocate DYNAMIC_ARRAY, on the heap, for every (insert-id
> >     generating) statement. Why?
> >
> > 2. You preallocate a buffer, but dynamic array cannot reallocate it, so
> >     on the 11'th element, it'll allocate *another* buffer and won't
> >     use yours.
> >
> > Better: put DYNAMIC_ARRAY in the THD, or (even better) allocate it with
> > thd->alloc. And let DYNAMIC_ARRAY do it's own memory management.
> Problem with thd->alloc is that MEM_ROOT of THD freed before we can send 
> the data. Idea of preallocation is that it is hardly exceed it in most 
> realistic scenario.

Why wouldn't you send your data before the the MEM_ROOT is freed?

Preallocation is fine, just don't do it with multi-malloc.
my_init_dynamic_array2 can preallocate internally.

and init=10, increment=100 looks rather weird.

> >> +    {
> >> +      if (insert_ids)
> >> +        my_free(insert_ids);
> >> +      insert_ids= NULL;
> >> +      return TRUE;
> >> +    }
> >> +    collect_auto_increment_increment= variables.auto_increment_increment;
> >> +  }
> >> +  return FALSE;
> >> +}
> >> +
> >> diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
> >> index 7c0c1b68a2b..20b775c9273 100644
> >> --- a/sql/sql_insert.cc
> >> +++ b/sql/sql_insert.cc
> >> @@ -1007,6 +1007,12 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
> >>       }
> >>       its.rewind();
> >>       iteration++;
> >> +
> >> +    if (!error && thd->bulk_param)
> >> +    {
> >> +      thd->collect_insert_id(table->file->insert_id_for_cur_row);
> >> +    }
> > 1. Why are you doing it here, and not per row, inside
> > `while ((values= its++))` loop?
> 
> It is what we need INSERT ID as if the command would be executed with 
> the parameters separately, i.e
> ID correspond to PARAMETER SET.

I'm sorry, I don't understand that.

> > 2. What about INSERT ... SELECT?
> It is not supported by ARRAY binding.

What does INSERT ID generation has to do with array binding?

> > 3. What about a insert-id generated in one statement but for many
> >     tables? (stored function, trigger, whatever)
> As above we need what usual INSERT send back to client in single execution.

Right, but if there's a trigger that inserts into another table, then
you might have two insert ids, for two different tables, in one
statement.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx