Hi, Oleksandr! On Nov 12, Oleksandr Byelkin wrote: > > On Nov 07, Oleksandr Byelkin wrote: > > > revision-id: 031efde365c674dbdbaada95aa6d42a4274db438 > > > (mariadb-10.2.18-65-g031efde365c) > > > parent(s): 89f948c766721a26e110bc9da0ca5ebc20f65112 > > > author: Oleksandr Byelkin > > > committer: Oleksandr Byelkin > > > timestamp: 2018-11-07 14:29:47 +0100 > > > message: > > > > > > MDEV-16217: Assertion `!table || (!table->read_set || > > bitmap_is_set(table->read_set, field_index))' failed in Field_num::get_date > > > > > > - clean up DEFAULT() to work only with default value and correctly print > > > itself. > > > - fix of DBUG_ASSERT about fields read/write > > > - fix of field marking for write based really on the > > > thd->mark_used_columns flag > > > > > diff --git a/sql/field.cc b/sql/field.cc > > > index caa84dc9932..6cd8940a893 100644 > > > --- a/sql/field.cc > > > +++ b/sql/field.cc > > > @@ -70,8 +70,21 @@ const char field_separator=','; > > > #define BLOB_PACK_LENGTH_TO_MAX_LENGH(arg) \ > > > ((ulong) ((1LL << MY_MIN(arg, 4) * 8) - 1)) > > > > > > -#define ASSERT_COLUMN_MARKED_FOR_READ DBUG_ASSERT(!table || > > (!table->read_set || bitmap_is_set(table->read_set, field_index))) > > > -#define ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED > > DBUG_ASSERT(is_stat_field || !table || (!table->write_set || > > bitmap_is_set(table->write_set, field_index) || (table->vcol_set && > > bitmap_is_set(table->vcol_set, field_index)))) > > > +// Column marked for read or the field set to read out or record[0] or > > [1] > > > +#define ASSERT_COLUMN_MARKED_FOR_READ \ > > > + DBUG_ASSERT(!table || \ > > > + (!table->read_set || \ > > > + bitmap_is_set(table->read_set, field_index) || \ > > > + (!(ptr >= table->record[0] && \ > > > + ptr < table->record[0] + table->s->reclength)))) > > > + > > > +#define ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED \ > > > + DBUG_ASSERT(is_stat_field || !table || \ > > > + (!table->write_set || \ > > > + bitmap_is_set(table->write_set, field_index) || \ > > > + (!(ptr >= table->record[0] && \ > > > + ptr < table->record[0] + table->s->reclength))) || \ > > > + (table->vcol_set && bitmap_is_set(table->vcol_set, > > field_index))) > > > > Do you need this ptr check in ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED ? > > I'd expect you only needing it in ASSERT_COLUMN_MARKED_FOR_READ. > > I'd prefer to have them symmetric
Right, but these macros already have non-symmetric names and non-symmetric definition - the second checks for vcol_set. The way you did it only makes the reader to pause and think in what case one can get `ptr` outside of table->record[0] for writes. Because your assert says "yes, it's ok to write into the column if ptr is outside of table->record[0]". If ptr cannot possibly be outside of record[0] for write, then it shouldn't be in the assert. > > > #define FLAGSTR(S,F) ((S) & (F) ? #F " " : "") > > > > > > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > > > index c282db42fdd..0deb5ec1362 100644 > > > --- a/sql/sql_base.cc > > > +++ b/sql/sql_base.cc > > > @@ -5737,7 +5737,7 @@ find_field_in_table_ref(THD *thd, TABLE_LIST > > *table_list, > > > TABLE *table= field_to_set->table; > > > if (thd->mark_used_columns == MARK_COLUMNS_READ) > > > bitmap_set_bit(table->read_set, field_to_set->field_index); > > > - else > > > + else if (thd->mark_used_columns == MARK_COLUMNS_WRITE) > > > bitmap_set_bit(table->write_set, field_to_set->field_index); > > > > what does it affect? > > > > enum enum_mark_columns > { MARK_COLUMNS_NONE, MARK_COLUMNS_READ, MARK_COLUMNS_WRITE}; > > MARK_COLUMNS_NONE really used in the code, I remember that something was > marked incorrectly (it is how I found it) but what it is too late to ask > after many month as fix is done. this code is inside of if (thd->mark_used_columns != MARK_COLUMNS_NONE) so I don't see what your change could possibly do. Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ 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