Hi Varun,
On Tue, Feb 07, 2017 at 10:19:50PM +0100, Sergei Golubchik wrote:
>
> I almost replied to your email with "it's impossible, Archive did not
> have HA_RECORD_MUST_BE_CLEAN_ON_WRITE in the table_flags(), no engine
> did. So removal could not have changed anything".
>
> But then I noticed that the code - by some old typo - had "&&" not "&".
> So it was always doing restore_record() in valgrind-aware binaries.
> For all engines. Apparently, it was never the original intention.
>
> Note that in release binaries HAVE_valgrind is not defined, and the
> record is not bzero-ed even in 10.1 and earlier versions. This is a bug
> in Archive engine, in all versions, not in 10.2 only. In should not
> store the uninitialized tail of VARCHAR columns.
>
Note that all this only happens in the "old" data format of the archive
tables. The new data format stores VARCHAR columns as variable-length data
and doesnt have this problem.
As for the old format, we can't change it.
I guess the solution is that ha_archive::pack_row_v1() should fill the padding
bytes in record_buffer->buffer (not in 'uchar *record' argument) with zeros.
I've did varchar packing for other storage engines, so I'll provide here some
pointers on how to do it:
* The data in "uchar *record" argument of pack_row_v1() is in the data format
that we call "table->record[0] format"
* In that data format, VARCHAR column has a fixed space.
It starts with this offset:
(field->ptr - table->record[0])
and takes field->pack_length_in_rec() bytes.
* field->is_real_null(record - table->record[0]) tells whether the field has a
NULL value. for NULLs, all varchar value is garbage
* for non-NULL values:
field_varchar->length_bytes shows how many bytes store the value length.
Then, this many bytes have the actual value. The rest is garbage.
* You only need to handle VARCHARs, field->real_type() == MYSQL_TYPE_VARCHAR
Hope that helps.
> On Feb 07, Varun Gupta wrote:
> > Hi,
> >
> > The commit is:
> >
> > commit c697ddc315429c56dec567b2fe85cfe5c03834ea
> > Author: Sergei Golubchik <s...@mariadb.org>
> > Date: Mon Dec 12 15:47:51 2016 +0100
> >
> > cleanup: remove unused handler table flag
> >
> > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
> >
> > index 901fd48..9c21cb7 100644
> > --- a/sql/sql_insert.cc
> > +++ b/sql/sql_insert.cc
> >
> > @@ -955,12 +955,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
> > be overwritten by fill_record() anyway (and fill_record() does
> > not
> > use default values in this case).
> > */
> > -#ifdef HAVE_valgrind
> > - if (table->file->ha_table_flags() &&
> > HA_RECORD_MUST_BE_CLEAN_ON_WRITE)
> > - restore_record(table,s->default_values); // Get empty record
> > - else
> > -#endif
> > - table->record[0][0]= share->default_values[0];
> > + table->record[0][0]= share->default_values[0];
> >
> > On Sun, Feb 5, 2017 at 8:10 PM, Sergei Golubchik <s...@mariadb.org> wrote:
> >
> > > Hi, Varun!
> > >
> > > On Feb 05, Varun Gupta wrote:
> > > > Hi,
> > > >
> > > > Here is the issue https://jira.mariadb.org/browse/MDEV-11645 on which I
> > > > need suggestions
> > > >
> > > > The observations made are:
> > > >
> > > > - in 10.1, varchar endspace was bzero-ed
> > > > archive depended on it to not pack garbage
> > > >
> > > > - in 10.2, varchar endspace is not bzero-ed
> > > > which breaks the archive SE.
> > > >
> > > > Was this change intended?
> > > >
> > > > If yes, then we should update ha_archive to fill the garbage parts
> > > > of the buffer with zeroes that will cause it to be (slightly?)
> > > > slower but there's no other way.
> > >
> > > Could you find what commit caused it? git bisect is quite useful for
> > > that.
> > >
BR
Sergei
--
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
