Skip to content

Commit

Permalink
MDEV-19385: Inconsistent definition of dtuple_get_nth_v_field()
Browse files Browse the repository at this point in the history
The accessor dtuple_get_nth_v_field() was defined differently between
debug and release builds in MySQL 5.7.8 in
mysql/mysql-server@c47e175
and a debug assertion to document or enforce the questionable assumption
tuple->v_fields == &tuple->fields[tuple->n_fields] was missing.

This was apparently no problem until MDEV-11369 introduced instant
ADD COLUMN to MariaDB Server 10.3. With that work present, in one
test case, trx_undo_report_insert_virtual() could in release builds
fetch the wrong value for a virtual column.

We replace many of the dtuple_t accessors with const-preserving
inline functions, and fix missing or misleadingly applied const
qualifiers accordingly.
  • Loading branch information
dr-m committed May 3, 2019
1 parent 3db94d2 commit ce19598
Show file tree
Hide file tree
Showing 21 changed files with 168 additions and 358 deletions.
4 changes: 2 additions & 2 deletions storage/innobase/data/data0data.cc
Expand Up @@ -37,7 +37,7 @@ Created 5/30/1994 Heikki Tuuri
/** Dummy variable to catch access to uninitialized fields. In the
debug version, dtuple_create() will make all fields of dtuple_t point
to data_error. */
byte data_error;
ut_d(byte data_error);
#endif /* UNIV_DEBUG */

/** Compare two data tuples.
Expand Down Expand Up @@ -416,7 +416,7 @@ dfield_print_also_hex(
break;
}

data = static_cast<byte*>(dfield_get_data(dfield));
data = static_cast<const byte*>(dfield_get_data(dfield));
/* fall through */

case DATA_BINARY:
Expand Down
3 changes: 1 addition & 2 deletions storage/innobase/fts/fts0fts.cc
Expand Up @@ -3366,12 +3366,11 @@ fts_fetch_doc_from_tuple(
const dict_field_t* ifield;
const dict_col_t* col;
ulint pos;
dfield_t* field;

ifield = dict_index_get_nth_field(index, i);
col = dict_field_get_col(ifield);
pos = dict_col_get_no(col);
field = dtuple_get_nth_field(tuple, pos);
const dfield_t* field = dtuple_get_nth_field(tuple, pos);

if (!get_doc->index_cache->charset) {
get_doc->index_cache->charset = fts_get_charset(
Expand Down
23 changes: 12 additions & 11 deletions storage/innobase/gis/gis0geo.cc
@@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 2013, 2015, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2019, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -68,9 +69,9 @@ static
int
rtree_add_point_to_mbr(
/*===================*/
uchar** wkb, /*!< in: pointer to wkb,
const uchar** wkb, /*!< in: pointer to wkb,
where point is stored */
uchar* end, /*!< in: end of wkb. */
const uchar* end, /*!< in: end of wkb. */
uint n_dims, /*!< in: dimensions. */
uchar byte_order, /*!< in: byte order. */
double* mbr) /*!< in/out: mbr, which
Expand Down Expand Up @@ -108,9 +109,9 @@ static
int
rtree_get_point_mbr(
/*================*/
uchar** wkb, /*!< in: pointer to wkb,
const uchar** wkb, /*!< in: pointer to wkb,
where point is stored. */
uchar* end, /*!< in: end of wkb. */
const uchar* end, /*!< in: end of wkb. */
uint n_dims, /*!< in: dimensions. */
uchar byte_order, /*!< in: byte order. */
double* mbr) /*!< in/out: mbr,
Expand All @@ -127,9 +128,9 @@ static
int
rtree_get_linestring_mbr(
/*=====================*/
uchar** wkb, /*!< in: pointer to wkb,
const uchar** wkb, /*!< in: pointer to wkb,
where point is stored. */
uchar* end, /*!< in: end of wkb. */
const uchar* end, /*!< in: end of wkb. */
uint n_dims, /*!< in: dimensions. */
uchar byte_order, /*!< in: byte order. */
double* mbr) /*!< in/out: mbr,
Expand Down Expand Up @@ -158,9 +159,9 @@ static
int
rtree_get_polygon_mbr(
/*==================*/
uchar** wkb, /*!< in: pointer to wkb,
const uchar** wkb, /*!< in: pointer to wkb,
where point is stored. */
uchar* end, /*!< in: end of wkb. */
const uchar* end, /*!< in: end of wkb. */
uint n_dims, /*!< in: dimensions. */
uchar byte_order, /*!< in: byte order. */
double* mbr) /*!< in/out: mbr,
Expand Down Expand Up @@ -195,9 +196,9 @@ static
int
rtree_get_geometry_mbr(
/*===================*/
uchar** wkb, /*!< in: pointer to wkb,
const uchar** wkb, /*!< in: pointer to wkb,
where point is stored. */
uchar* end, /*!< in: end of wkb. */
const uchar* end, /*!< in: end of wkb. */
uint n_dims, /*!< in: dimensions. */
double* mbr, /*!< in/out: mbr. */
int top) /*!< in: if it is the top,
Expand Down Expand Up @@ -297,7 +298,7 @@ stored in "well-known binary representation" (wkb) format.
int
rtree_mbr_from_wkb(
/*===============*/
uchar* wkb, /*!< in: wkb */
const uchar* wkb, /*!< in: wkb */
uint size, /*!< in: size of wkb. */
uint n_dims, /*!< in: dimensions. */
double* mbr) /*!< in/out: mbr, which must
Expand Down
9 changes: 5 additions & 4 deletions storage/innobase/gis/gis0rtree.cc
@@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2019, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -64,7 +65,7 @@ rtr_page_split_initialize_nodes(
page_t* page;
ulint n_uniq;
ulint len;
byte* source_cur;
const byte* source_cur;

block = btr_cur_get_block(cursor);
page = buf_block_get_frame(block);
Expand Down Expand Up @@ -104,7 +105,7 @@ rtr_page_split_initialize_nodes(
}

/* Put the insert key to node list */
source_cur = static_cast<byte*>(dfield_get_data(
source_cur = static_cast<const byte*>(dfield_get_data(
dtuple_get_nth_field(tuple, 0)));
cur->coords = reserve_coords(buf_pos, SPDIMS);
rec = (byte*) mem_heap_alloc(
Expand Down Expand Up @@ -1874,11 +1875,11 @@ rtr_estimate_n_rows_in_range(
ulint dtuple_f_len MY_ATTRIBUTE((unused));
rtr_mbr_t range_mbr;
double range_area;
byte* range_mbr_ptr;

dtuple_field = dtuple_get_nth_field(tuple, 0);
dtuple_f_len = dfield_get_len(dtuple_field);
range_mbr_ptr = reinterpret_cast<byte*>(dfield_get_data(dtuple_field));
const byte* range_mbr_ptr = static_cast<const byte*>(
dfield_get_data(dtuple_field));

ut_ad(dtuple_f_len >= DATA_MBR_LEN);
rtr_read_mbr(range_mbr_ptr, &range_mbr);
Expand Down
6 changes: 2 additions & 4 deletions storage/innobase/gis/gis0sea.cc
Expand Up @@ -1631,15 +1631,13 @@ rtr_get_mbr_from_tuple(
{
const dfield_t* dtuple_field;
ulint dtuple_f_len;
byte* data;

dtuple_field = dtuple_get_nth_field(dtuple, 0);
dtuple_f_len = dfield_get_len(dtuple_field);
ut_a(dtuple_f_len >= 4 * sizeof(double));

data = static_cast<byte*>(dfield_get_data(dtuple_field));

rtr_read_mbr(data, mbr);
rtr_read_mbr(static_cast<const byte*>(dfield_get_data(dtuple_field)),
mbr);
}

/****************************************************************//**
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/handler/ha_innodb.cc
Expand Up @@ -21784,7 +21784,7 @@ void innobase_free_row_for_vcol(VCOL_STORAGE *storage)
to store the value in passed in "my_rec" */
dfield_t*
innobase_get_computed_value(
const dtuple_t* row,
dtuple_t* row,
const dict_v_col_t* col,
const dict_index_t* index,
mem_heap_t** local_heap,
Expand Down

0 comments on commit ce19598

Please sign in to comment.