page_is_corrupted(): Do not allocate the buffers from stack,
but from the heap, in xb_fil_cur_open().
row_quiesce_write_cfg(): Issue one type of message when we
fail to create the .cfg file.
update_statistics_for_table(), read_statistics_for_table(),
delete_statistics_for_table(), rename_table_in_stat_tables():
Use a common stack buffer for Index_stat, Column_stat, Table_stat.
ha_connect::FileExists(): Invoke push_warning_printf() so that
we can avoid allocating a buffer for snprintf().
translog_init_with_table(): Do not duplicate TRANSLOG_PAGE_SIZE_BUFF.
Let us also globally enable the GCC 4.4 and clang 3.0 option
-Wframe-larger-than=16384 to reduce the possibility of introducing
such stack overflow in the future. For RocksDB and Mroonga we relax
these limits.
Reviewed by: Vladislav Lesin
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>
- fix several Windows-specific "variable set but not used",
or "variable unused" warnings.
- correctly initialize std::atomic_flag (ATOMIC_FLAG_INIT)
- fix Ninja build for spider on Windows
- adjust check for sizeof(MYSQL) for Windows compilers
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.
Each spider connection is identified with a connection key, which is
an encoding of the backend parameters.
The first byte of the key is by default 0, and in rare circumstances
it is changed to a different value: when semi_table_lock is set to 1;
and when using casual read. When this happens, often a new connection
is created with the new key. Neither case is useful: the description
of semi_table_lock has nothing to do with creation of new connections
and the parameter itself was deprecated for 10.7+ (MDEV-28829) and
marked for deletion (MDEV-28830); while new threads created by
non-zero spider_casual_read causes only threads to be idle, thus not
achieving any gain, see MDEV-26151, and the param has also been
deprecated in 11.5+ (MDEV-31789). The relevant code adds unnecessary
complexity to the spider code. This change does not reduce
parallelism, because already when bgs mode is on a background thread
is created per partition, and there is no evidence spider creates
multiple threads for one partition. If the needs of such cases arise
it will be a separate issue.
The conn_kind, which stands for "connection kind", is no longer useful
because the HandlerSocket support is deleted and Spider now has only
one connection kind, SPIDER_CONN_KIND_MYSQL. Remove conn_kind and
related code.
Signed-off-by: Yuchen Pei <yuchen.pei@mariadb.com>
Reviewed-by: Nayuta Yanagisawa <nayuta.yanagisawa@mariadb.com>
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 mismatch occurs on the function calls as in the sql/sql_udf.h the
types of "error" and "is_null" are unsigned char rather than char.
This is corrected for the udf functions:
* spider_direct_sql
* spider_direct_bg_sql
* spider_flush_table_mon_cache
* spider_copy_tables
* spider_ping_table
Reviewer: Yuchen Pei
