In spider_db_mbase_util::print_item_func(), if the sql item_func has
an UNKNOWN_FUNC type, by default the spider group by handler (gbh)
transform infix to prefix. But regexp should remain infix, so we add
an if condition to account for this.
This will avoid issues like MDEV-32486
IDs used in
- spider_alloc_calc_mem_init()
- spider_string::init_calc_mem()
- spider_malloc()
- spider_bulk_alloc_mem()
- spider_bulk_malloc()
Spider GBH's query rewrite of table joins is overly complex and
error-prone. We replace it with something closer to what
dbug_print() (more specifically, print_join()) does, but catered to
spider.
More specifically, we replace the body of
spider_db_mbase_util::append_from_and_tables() with a call to
spider_db_mbase_util::append_join(), and remove downstream append_X
functions.
We make it handle const tables by rewriting them as (select 1). This
fixes the main issue in MDEV-26247.
We also ban semijoin from spider gbh, which fixes MDEV-31645 and
MDEV-30392, as semi-join is an "internal" join, and "semi join" does
not parse, and it is different from "join" in that it deduplicates the
right hand side
Not all queries passed to a group by handler are valid (MDEV-32273),
for example, a join on expr may refer outer fields not in the current
context. We detect this during the handler creation when walking the
join. See also gbh_outer_fields_in_join.test.
It also skips eliminated tables, which fixes MDEV-26193.
Spider gbh query rewrite should get table for fields in a simple way.
Add a method spider_fields::find_table that searches its table holders
to find table for a given field. This way we will be able to get rid
of the first pass during the gbh creation where field_chains and
field_holders are created.
We also check that the field belongs to a spider table while walking
through the query, so we could remove
all_query_fields_are_query_table_members(). However, this requires an
earlier creation of the table_holder so that tables are added before
checking. We do that, and in doing so, also decouple table_holder and
spider_fields
Remove unused methods and fields. Add comments.
Two methods from spider_fields. There are probably more of these
conn_holder related methods that can be removed
reappend_tables_part()
reappend_tables()
The MDEV-29693 conflict resolution is from Monty, as well as is
a bug fix where ANALYZE TABLE wrongly built histograms for
single-column PRIMARY KEY.
Also includes a fix for safe_malloc error reporting.
Other things:
- Copied main.log_slow from 10.4 to avoid mtr issue
Disabled test:
- spider/bugfix.mdev_27239 because we started to get
+Error 1429 Unable to connect to foreign data source: localhost
-Error 1158 Got an error reading communication packets
- main.delayed
- Bug#54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED
This part is disabled for now as it fails randomly with different
warnings/errors (no corruption).
The direct aggregate mechanism sems to be only intended to work when
otherwise a full table scan query will be executed from the spider
node and the aggregation done at the spider node too. Typically this
happens in sub_select(). In the test spider.direct_aggregate_part
direct aggregate allows to send COUNT statements directly to the data
nodes and adds up the results at the spider node, instead of iterating
over the rows one by one at the spider node.
By contrast, the group by handler (GBH) typically sends aggregated
queries directly to data nodes, in which case DA does not improve the
situation here.
That is why we should fix it by disabling DA when GBH is used.
There are other reasons supporting this change. First, the creation of
GBH results in a call to change_to_use_tmp_fields() (as opposed to
setup_copy_fields()) which causes the spider DA function
spider_db_fetch_for_item_sum_funcs() to work on wrong items. Second,
the spider DA function only calls direct_add() on the items, and the
follow-up add() needs to be called by the sql layer code. In
do_select(), after executing the query with the GBH, it seems that the
required add() would not necessarily be called.
Disabling DA when GBH is used does fix the bug. There are a few
other things included in this commit to improve the situation with
spider DA:
1. Add a session variable that allows user to disable DA completely,
this will help as a temporary measure if/when further bugs with DA
emerge.
2. Move the increment of direct_aggregate_count to the spider DA
function. Currently this is done in rather bizarre and random
locations.
3. Fix the spider_db_mbase_row creation so that the last of its row
field (sentinel) is NULL. The code is already doing a null check, but
somehow the sentinel field is on an invalid address, causing the
segfaults. With a correct implementation of the row creation, we can
avoid such segfaults.
The original test in the report of MDEV-31463 is contrived and
nondeterministic, causing MDEV-31586. We update the test to make it
more directly addresses the underlying cause of MDEV-31463, namely
errors from queries sent to the data node not consumed when trying to
set lock wait timeout. This is achieved through the debug sync
facility.
When setting the server lockwait timeout, spider should do some basic
checks first, like whether the remote server is still reachable. So
instead of directly calling spider_db_mbase::exec_query(), it should
call spider_db_query().
The reset of the lock wait timeout does not need to do such checks,
because they happen after the successfully setting the lock wait
timeout, implying the checks have been passed already.
spider_db_mbase_util::open_item_func() is a monster function.
It is difficult to maintain while it is expected that we need to
modify it when a new SQL function or a new func_type is added.
We split the function into two distinct functions: one handles the
case of str != NULL and the other handles the case of str == NULL.
This refactoring was done in a conservative way because we do not
have comprehensive tests on the function.
It also fixes MDEV-29447 and MDEV-31338 where field items that are
arguments of a func item may be used before created / initialised.
Note this commit is adapted from a patch by Nayuta for MDEV-26285.
Set the lock wait timeout to 1 beforehand, and reset it afterwards, to
avoid lock conflict caused by opening the same table twice in case of
self-reference.
Extracted out common subroutines, gave more meaningful names etc,
added comments etc.
Also:
- Documented active servers load balancing reads, and other fields in
SPIDER_SHARE etc.
- Removed commented out code
- Documented and refactored self-reference check
- Removed some unnecessary functions
- Renamed unhelpful roop_count
- Refactored spider_get_{sts,crd}, where we turn get_type into an enum
- Cleaned up spider_mbase_handler::show_table_status() and
spider_mbase_handler::show_index()
