Skip to content

Commit 132b094

Browse files
author
Chaithra Gopalareddy
committed
Bug#35894664: Assertion in Bitmap<64>::is_set(uint)
when forcing a spatial index scan. Bug#36361834: Incorrect result of getting data from a spatial index on column with SRID Bug#36366621: MySQL crash when explaining a force a spatial index containing column with SRID Bug#36361834: While setting up keys that could be used for covering index scans, a "geometry" keypart makes an index not covering. However, in case of innodb, a secondary index is always extended with primary key. While setting up the extended keypart spatial index is marked as covering. With force index in effect, spatial index is the only available index for optimizer to choose from. It picks "index-skip-scan" for the table. However, innodb does not support sequential scans on spatial indexes. This leads to wrong results with no rows retrieved. Bug#35894664 and Bug#36366621: When choosing covering index scans, optimizer picks the shortest covering key available. However, for the case where spatial index is the only available option, it fails to pick any index to do the index scan which leads to problems later. Solution to all these issues is that spatial indexes should not be marked as indexes that could be used for index only reads. There are a few inconsistencies in the code w.r.t this. So we try to correct them. While setting up keyparts, "geometry" field makes the index not covering but having an extended keypart which is not a geometry field makes the index covering which should not be the case. So we now disable spatial index for "index-only" reads irrespective of the extended keyparts. When introducing the change to disable "index-only" reads for indexes having "geometry" fields as part of Bug#19806196, "spatial index" was not expected to be part of "prefix keys" as well. However, it was enabled after some of the checks were changed/moved later. This also led to an inconsistency. E.g if the indexed "geometry" field is used as part of the query, even though originally the spatial index was indeed marked as covering (because of the extended keyparts), while setting up the "geometry" field (in set_field()), prefix keys for the field are removed from the "covering_keys" keymap which made the spatial index as not covering. This is also the reason we do not see problems for cases where the indexed field is part of the query. So we fix this inconsistency by disabling "spatial indexes" to be considered as prefix keys. There was a check to disable picking a "spatial index" to do covering index scan late in optimization phase. This is removed now as it is taken care much earlier. Change-Id: I44c04f79f35a270087a01c962f9ad35f5e3ad534
1 parent aff77ab commit 132b094

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

sql/sql_optimizer.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1963,14 +1963,18 @@ uint find_shortest_key(TABLE *table, const Key_map *usable_keys) {
19631963
if (nr == usable_clustered_pk) continue;
19641964
if (usable_keys->is_set(nr)) {
19651965
/*
1966-
Can not do full index scan on rtree index because it is not
1967-
supported by Innodb, probably not supported by others either.
1966+
Cannot do full index scan on rtree index. It is not supported by
1967+
Innodb as it's rtree index does not store data, but only the
1968+
minimum bouding box (maybe makes sense only for geometries of
1969+
type POINT). Index scans on rtrees are probabaly not supported
1970+
by other storage engines either.
19681971
A multi-valued key requires unique filter, and won't be the most
19691972
fast option even if it will be the shortest one.
19701973
*/
19711974
const KEY &key_ref = table->key_info[nr];
1972-
assert(!(key_ref.flags & HA_MULTI_VALUED_KEY));
1973-
if (key_ref.key_length < min_length && !(key_ref.flags & HA_SPATIAL)) {
1975+
assert(!(key_ref.flags & HA_MULTI_VALUED_KEY) &&
1976+
!(key_ref.flags & HA_SPATIAL));
1977+
if (key_ref.key_length < min_length) {
19741978
min_length = key_ref.key_length;
19751979
best = nr;
19761980
}

sql/table.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -738,22 +738,25 @@ void setup_key_part_field(TABLE_SHARE *share, handler *handler_file,
738738

739739
const bool full_length_key_part =
740740
field->key_length() == key_part->length && !field->is_flag_set(BLOB_FLAG);
741+
const bool is_spatial_key = Overlaps(keyinfo->flags, HA_SPATIAL);
741742
/*
742743
part_of_key contains all non-prefix keys, part_of_prefixkey
743744
contains prefix keys.
744745
Note that prefix keys in the extended PK key parts
745746
(part_of_key_not_extended is false) are not considered.
746-
Full-text keys are not considered prefix keys.
747+
Full-text and spatial keys are not considered prefix keys.
747748
*/
748749
if (full_length_key_part || Overlaps(keyinfo->flags, HA_FULLTEXT)) {
749750
field->part_of_key.set_bit(key_n);
750751
if (part_of_key_not_extended)
751752
field->part_of_key_not_extended.set_bit(key_n);
752-
} else if (part_of_key_not_extended) {
753+
} else if (part_of_key_not_extended && !is_spatial_key) {
753754
field->part_of_prefixkey.set_bit(key_n);
754755
}
756+
// R-tree indexes do not allow index scans and therefore cannot be
757+
// marked as keys for index only access.
755758
if ((handler_file->index_flags(key_n, key_part_n, false) & HA_KEYREAD_ONLY) &&
756-
field->type() != MYSQL_TYPE_GEOMETRY) {
759+
!is_spatial_key) {
757760
// Set the key as 'keys_for_keyread' even if it is prefix key.
758761
share->keys_for_keyread.set_bit(key_n);
759762
}

0 commit comments

Comments
 (0)