MDEV-26247 [demo] address the first pass during gbh creation
Spider GBH does two passes of spider_db_print_item_type() for all
items in the query. The first pass during the GBH creation has
str==NULL, to construct a chain of fields pointing to table alias and
the correct associated ha_spider. The second pass during the
init_scan() walks the query again to translate to the query for remote
tables. See also MDEV-29447.
After re-implementing rewriting the FROM part of the query, we also
have to update the corresponding first pass part that processes FROM.
For the second pass to work, the two passes have to walk the queries
so that the items are in the exact same order. But the simple update
of the first pass in this patch seems to break that order: the test
spider.direct_right_left_join_nullable fails an assertion of the
order.
There are two ways to address this:
1. Just do one pass of walking the item. The complexity should be the
same, up to a constant
2. Figure out why the order is wrong now, and fix it.
Of the two ideas, 1 is better, because it can significantly simplify
the code by getting rid of the first pass and the different behaviours
of the spider item printing functions depending on the nullness of
str, and it will also make it much easier to debug with ordering
issues, because we don't need to go back and forth between the two
passes, and it will make the code easier to maintain.
MDEV-26247 [PoC] Re-implement spider gbh query rewrite of tables
Spider GBH's query rewrite of table joins is overly complex and
error-prone. In this PoC commit, we explore replacing it with
something closer to what dbug_print() (more specifically,
print_join()) does, but catered to spider. It seems to be working
well, based on two examples I have tested:
select * from t3 left join t1 on t3.a = t1.a left join t2 on t3.a = t2.a;
select * from t1 left join t2 on t1.a = t2.a right join t3 on t3.a = t1.a;
Note that we have not removed the old functions yet, and it does not
support const tables or (presumably) eliminated tables.
However, it fails the second example due to issues in *item* printing.
Unfortunately, these issues already exist in the spider GBH without
the change in this commit (i.e. not a regression), so we have to fix
that too, but perhaps as a separate task... See below about the
failure
--8<---------------cut here---------------start------------->8---
select * from t1 left join t2 on t1.a = t2.a right join t3 on t3.a = t1.a
select t0.`a` `a`,t1.`a` `a`,t2.`a` `a` from `auto_test_remote`.`t3` t2 left join (`auto_test_remote`.`t1` t0 left join `auto_test_remote`.`t2` t1 on (t1.`a` = t2.`a`)) on (t0.`a` = t2.`a`) where 1
select t0.`a` `a`,t1.`a` `a`,t2.`a` `a` from `auto_test_remote`.`t3` t2 left join ( left join `auto_test_remote`.`t2` t1 on (t1.`a` = t2.`a`) join `auto_test_remote`.`t1` t0) on (t0.`a` = t2.`a`) where 1
--8<---------------cut here---------------end--------------->8---
MDEV-29502 Fix some issues with spider direct aggregate
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.
MDEV-31117 clean up spider connection info parsing
Spider connection string is a comma-separated parameter definitions,
where each definition is of the form "<param_title> <param_value>",
where <param_value> is quote delimited on both ends, with backslashes
acting as an escaping prefix.
The code however treated param title the same way as param value when
assigning, and have nonsensical fields like delim_title_len and
delim_title. We remove these.
We also clean up the spider comment connection string parsing,
including:
- Factoring out some code from the parsing function
- Rewriting the struct `st_spider_param_string_parse`, including
replacing its messy methods with cleaner ones
- And any necessary changes caused by the above changes
Non-minus-one var value overrides table param overrides default value.
Before MDEV-27169, unspecified var value is -1. So if the user sets
both the var to be a value other than -1 and the table param, the var
value will prevail, which is incorrect.
After MDEV-27169, unspecified var value is default value. So if the
user does not set the var but sets the table param, the default value
will prevail, which is even more incorrect.
This patch fixes it so that table param, if specified, always
overrides var value, and the latter if not specified or set to -1,
falls back to the default value
We achieve this by replacing all such overriding in spd_param.cc with
macros that override in the correct way, and removing all the
"overriding -1" lines involving table params in
spider_set_connect_info_default() except for those table params not
defined as sysvar/thdvar in spd_params.cc
We also introduced macros for non-overriding sysvar and thdvar, so
that the code is cleaner and less error-prone
In server versions where MDEV-27169 has not been applied, we also
backport the patch, that is, replacing -1 default values with real
default values
In server versions where MDEV-28006 has not been applied, we do the
same for udf params
Fix spider init bugs (MDEV-22979, MDEV-27233, MDEV-28218) while
preventing regression on old ones (MDEV-30370, MDEV-29904)
Two things are changed:
First, Spider initialisation is made fully synchronous, i.e. it no
longer happens in a background thread. Adapted from the original fix
by nayuta for MDEV-27233. This change itself would cause failure when
spider is initialised early, by plugin-load-add, due to dependency on
Aria and udf function creation, which are fixed in the second and
third parts below. Requires SQL Service, thus porting earlier versions
requires MDEV-27595
Second, if spider is initialised before udf_init(), create udf by
inserting into `mysql.func`, otherwise do it by `CREATE FUNCTION` as
usual. This change may be generalised in MDEV-31401.
Also factor out some clean-up queries from deinit_spider.inc for use
of spider init tests.
A minor caveat is that early spider initialisation will fail if the
server is bootstrapped for the first time, due to missing `mysql`
database which needs to be created by the bootstrap script.
