maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sachin!

On Mar 12, Sachin Setiya wrote:
> revision-id: b5d2e8577ef (mariadb-10.4.3-62-gb5d2e8577ef)
> parent(s): 202d1b0c897
> author: Sachin Setiya <sachin.setiya@xxxxxxxxxxx>
> committer: sachin <sachin.setiya@xxxxxxxxxxx>
> timestamp: 2019-03-12 19:29:58 +0530
> message:
> 
> MDEV-18889 Long unique on virtual fields crashes server
> 
> When we receive record from ha_index_read_map it does not have vfield
> values. So we need to update it.
> 
> diff --git a/sql/handler.cc b/sql/handler.cc
> index 0746f8a81fc..c4f81c6432e 100644
> --- a/sql/handler.cc
> +++ b/sql/handler.cc
> @@ -6522,6 +6522,26 @@ static int wsrep_after_row(THD *thd)
>  }
>  #endif /* WITH_WSREP */
>  
> +/*
> +  Long Unique can have virtual fields. But when we receive record from
> +  ha_index_read_map it does not have vfield values. So we need to update it.
> +  IN   table       Table Object
> +  IN   handler     Handler Object
> +  OUT  updated     Set to true on successful update

Doxygen style for new comments, please.

And feel free to rewrite old comments to use Doxygen style whenever you
need to edit them anyway.

> +*/
> +static void update_table_vcol(TABLE *table, handler *h, bool& updated)
> +{
> +  uint length= table->s->reclength;
> +  uchar temp_record[length];

Not a good idea:

1. you'll get compiler warnings for using VLA (-Wvla: Warn if a
   variable-length array is used")
2. length can be rather large, stack can be rather small.
3. two big memcpy's per row read.

Two other options:

1. use field->move(check_unique_buf - record[0]) to move field pointers
   to check_unique_buf. You can do it once, at the beginning and
   restore them at the end, no need to do it per read.
   See TABLE::move_fields()

2. Copy record[0] to check_unique_buf(). And read potential duplicates
   into record[0]. Then you won't need to do anything special at all,
   ha_index_read_map() will update vcols automatically.
   Just don't forget to restore record[0] at the end.
   See store_record(), restore_record().

I think the second approach might be easier.

> +  memcpy(temp_record, table->record[0], length);
> +  restore_record(table, check_unique_buf);
> +  if (!table->update_virtual_fields(h, VCOL_UPDATE_FOR_READ))
> +    updated= true;
> +  store_record(table, check_unique_buf);
> +  memcpy(table->record[0], temp_record,  length);
> +
> +}
> +
Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx