With view protocol collation_connection is reset in mysql_make_view in
the "SELECT * FROM mysqltest_tmp_v" query. In the case of
spider/bugfix.mdev_33434, it is reset to latin1_swedish_ci, with the
latin1 charset.
This results in no conversion needed since it is the same as
character_set_client and the corresponding argument in the udf remains
unchanged, with "dummy" srv value. Thus the reported error is
1477: 'The foreign server name you are trying to reference does not exist. Data source error: dummy'
Without view protocol, the character_set_connection ucs2 setting in
the test survives, and the conversion results in empty connection
parameters, and the reported error is 1429
ER_CONNECT_TO_FOREIGN_DATA_SOURCE
This failure is irrelevant to the test, or to spider at all. Therefore
we disable view protocol for the statement.
In spider/bugfix.mdev_29352, with flush tables with read lock,
statements blocked in THD::has_read_only_protection() by checking
THD::global_read_lock could result in view protocol to "hang" waiting
for acquiring mdl in another THD.
In spider/bugfix.mdev_34555, within an XA transaction, statements
blocked by trans_check() by checking thd->transaction->xid_state could
result in view protocol to "hang" for the same reason.
Therefore we disable view protocol for relevant statements in these
tests.
Spider tables do not support SELECT SQL_CALC_FOUND_ROWS and the
correct test output is a coincidence. Debugging shows that the
limit_found_rows field was last updated in an unrelated statement:
SELECT STRAIGHT_JOIN a.a, a.b, date_format(b.c, '%Y-%m-%d %H:%i:%s')\nFROM ta_l a, tb_l b WHERE a.a = b.a ORDER BY a.a
As a byproduct, this fixes the "wrong found_rows() results" when
running these tests with view protocol.
The failure is caused by exec $stmt where $stmt has two queries.
mtr with view protocol transforms the first query into a view, leaving
the second query executed in the usual way. mtr being oblivious about
the second query then does not handle its results, resulting in
CR_COMMANDS_OUT_OF_SYNC. We disable view protocol for such edge cases.
After fixing these "Failed to drop view: 0: " further failures emerge
from two of the tests, which are the same problem as MDEV-36454, so we
fix them to by disabling view protocol for the relevant SELECTs.
With view protocol, a SELECT statement is transformed into two
statements:
1. CREATE OR REPLACE VIEW mysqltest_tmp_v AS SELECT ...
2. SELECT * FROM mysqltest_tmp_v
The first statement reconstructed the query, which is executed in the
second statement.
The reconstruction often replaces aliases in ORDER BY by the original
item.
For example, in the test spider/bugfix.mdev_29008 the query
SELECT MIN(t2.a) AS f1, t1.b AS f2 FROM tbl_a AS t1 JOIN tbl_a AS t2 GROUP BY f2 ORDER BY f1, f2;
is transformed to
"select min(`t2`.`a`) AS `f1`,`t1`.`b` AS `f2` from (`auto_test_local`.`tbl_a` `t1` join `auto_test_local`.`tbl_a` `t2`) group by `t1`.`b` order by min(`t2`.`a`),`t1`.`b`"
In such cases, spider constructs different queries to execute at the
data node. So we disable view protocol for such queries.
With view protocol, often during optimization, the GBH is not created
because join->tables_list is the view mysqltest_tmp_v which has MEMORY
as engine which does not have GBH implemented.
In such cases, if without view protocol the test takes a path that
does create a spider GBH, the resulting queries sent to the data node
often differ.
Therefore we disable view protocol for these statements.
Spider needs to lock the spider table when executing the udf, but the
server layer would have already locked tables in view protocol because
it transforms the query:
select spider_copy_table('t', 0, 1)
to two queries
create or replace view mysqltest_tmp_v as select
spider_copy_table('t', 0, 1);
select * from mysqltest_tmp_v;
So spider justifiably errors out in this case by checking on
thd->derived_tables and thd->locks in spider_copy_tables_body()
If one of the selected field is a MIN or MAX and it has been optimized
into a constant, it is not added to the temp table used by a group by
handler (GBH). The GBH therefore cannot store results to this missing
field.
On the other hand, when SELECTing from a view or a derived table,
TMP_TABLE_ALL_COLUMNS is set. If the query has no group by or order
by, an Item_temptable_field is created for this MIN/MAX field and
added to the JOIN. Since the GBH could not store results to the
corresponding field in the temp table, the value of this
Item_temptable_field remains NULL. And the NULL value is passed to the
record, then the temp row, and finally output as the (wrong) result.
To fix this, we opt to not creating a spider GBH when a view or
derived table is involved.
This fixes spider/bugfix.mdev_26345 for --view-protocol
Also fixed a comment:
TABLE_LIST::belong_to_derived is NULL if the table belongs to a
derived table that has non-MERGE type.
Running mtr --view-protocol transforms SELECT statements to a CREATE
OR REPLACE VIEW of the statement, followed by SELECT from the view.
When thus when spider tests check the query log for select statements,
it often output a different one with --view-protocol compared to
without.
By adding disable/enable_view_protocol pairs to these statements. Most
of these statements are surrounded by existing
disable/enable_ps[2]_protocol pairs.
Acked-by: Yuchen Pei <ycp@mariadb.com>
With --view-protocol, mtr transforms a SELECT query to two queries:
1. CREATE OR REPLACE VIEW mysqltest_tmp_v AS ...
2. SELECT * FROM mysqltest_tmp_v
where ... is the original query. Further mtr may run the first query
in a separate connection.
On the other hand if the data node is the same as the spider node,
spider_same_server_link is required for connection to the data node.
Therefore, for mtr --view-protocol tests often spider_same_server_link
needs to be set on both session and global levels. In this patch we
add the missing "SET GLOBAL spider_same_server_link=1" queries to
tests that fail with wrong results due to this issue.
It does not fix --view-protocol for all the affected tests, because
there are other issues fixed in subsequent patches.
In MDEV-26345 77ed235d50 a bitmap is
introduced to skip spider GBH SELECTed constant fields when storing
the results from the data node. Unfortunately this bitmap was not used
in all applicable calls. This patch fixes it. The test covers most of
the calls, with the exception of
spider_db_store_result_for_reuse_cursor(), which is not covered in
existing tests, because it is only called when limit_mode()==1, which
is not the case for any spider backend wrapper.
A spider_conn may outlive its associated ha_spider (in the field
queued_ping_spider) used for connecting to and pinging the data
node (a call to spider_db_ping(), guarded by the boolean field
queued_ping). In a call to ha_spider::close() (which is often preceded
with the deletion of the ha_spider itself), many cleanups happen,
including freeing the associated spider_share, which is used by the
spider_conn in spider_db_ping. Therefore it is necessary to reset both
the queued_ping_spider and queued_ping fields, so that any further
spider interaction with the data node will not trigger the call using
the ha_spider including its freed spider_share.
Also out of caution added an assert and internal error in case a
connection has not been established (the db_conn field of type
MYSQL * is NULL), and attempt to connect is skipped because both
queued_connect and queued_ping are false. Note that this unlikely (if
not impossible) scenario would not be a regression caused by this
change, as it strictly falls under the scenario of this bug.
This ensures that the error message is populated when the reading
fails with ER_NET_READ_INTERRUPTED, which at the client layer returns
without storing any error message as there is no corresponding CR_
error code.
Patch originally by Sergei Golubchik <serg@mariadb.com>
Update conn->queue_connect_share in spider_check_trx_and_get_conn to
avoid use-after-free.
There are two branches in spider_check_trx_and_get_conn, often called
at the beginning of a spider DML, depending on whether an update of
various spider fields is needed. If it is determined to be needed, the
updating may NULL the connections associated with the spider handler,
which subsequently causes a call to spider_get_conn() which updates
conn->queued_connect_share with the SPIDER_SHARE associated with the
spider handler.
We make it so that conn->queued_connect_share is updated regardless of
the branch it enters, so that it will not be a stale and potentially
already freed one.
The problems were that:
1) resources was freed "asimetric" normal execution in send_eof,
in case of error in destructor.
2) destructor was not called in case of SP for result objects.
(so if the last SP execution ended with error resorces was not
freeded on reinit before execution (cleanup() called before next
execution) and destructor also was not called due to lack of
delete call for the object)
Result cleanup() renamed to reset_for_next_ps_execution() to better
reflect function().
All result method revised and freeing resources made "symetric".
Destructor of result object called for SP.
Added skipped invalidation in case of error in insert.
Removed misleading naming of reset(thd) (could be mixed with
with reset()).
The limit of socket length on unix according to libc is 108, see
sockaddr_un::sun_path, but in the table it is a string of max length
64, which results in truncation of socket and failure to connect by
plugins using servers such as spider.
This is a fixup of MDEV-26345 commit
77ed235d50.
In MDEV-26345 the spider group by handler was updated so that it uses
the item_ptr fields of Query::group_by and Query::order_by, instead of
item. This was and is because the call to
join->set_items_ref_array(join->items1) during the execution stage,
just before the execution replaces the order-by / group-by item arrays
with Item_temptable_field.
Spider traverses the item tree during the group by handler (gbh)
creation at the end of the optimization stage, and decides a gbh could
handle the execution of the query. Basically spider gbh can handle the
execution if it can construct a well-formed query, executes on the
data node, and store the results in the correct places. If so, it will
create one, otherwise it will return NULL and the execution will use
the usual handler (ha_spider instead of spider_group_by_handler). To
that end, the general principle is the items checked for creation
should be the same items later used for query construciton. Since in
MDEV-26345 we changed to use the item_ptr field instead of item field
of order-by and group-by in query construction, in this patch we do
the same for the gbh creation.
The item_ptr field could be the uninitialised NULL value during the
gbh creation. This is because the optimizer may replace a DISTINCT
with a GROUP BY, which only happens if the original GROUP BY is empty.
It creates the artificial GROUP BY by calling create_distinct_group(),
which creates the corresponding ORDER object with item field aligning
with somewhere in ref_pointer_array, but leaving item_ptr to be NULL.
When spider finds out that item_ptr is NULL, it knows there's some
optimizer skullduggery and it is passed a query different from the
original. Without a clear contract between the server layer and the
gbh, it is better to be safe than sorry and not create the gbh in this
case.
Also add a check and error reporting for the unlikely case of item_ptr
changing from non-NULL at gbh construction to NULL at execution to
prevent server crash.
Also, we remove a check added in MDEV-29480 of order by items being
aggregate functions. That check was added with the premise that spider
was including auxiliary SELECT items which is referenced by ORDER BY
items. This premise was no longer true since MDEV-26345, and caused
problems such as MDEV-29546, which was fixed by MDEV-26345.
