20 Mar
2017
20 Mar
'17
6:31 p.m.
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@mariadb.org