Repository navigation
MDEV-31342 Fix I_S fast-path follow-up bugs - #5752
Conversation
gkodinov
added
the
External Contribution
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
I believe we need to simplify the whole thing here.
Right now there's multiple Jiras and multiple pull requests. There's "main" Jira that has the base. And then multiple other jiras each holding some base and then an addon diff. And all of these things have different statuses.
I would suggest this whole feature be brought down to one jira and one pull request. Possibly with many commits in it as needed.
But it's kind of odd to use multiple jiras to track a single feature work IMHO. And the statuses discrepancies are not helping.
Please discuss this with your mentor. And consider my proposal above. Or give me good arguments on why this won't work.
Now on the immediate review:
There are test suite failures with this one. Are you sure it's ready for review?
And also there are suggested changes by Sanja in the jira that I do not see.
7060de8 to
db259f3
Compare
|
This is now one PR for MDEV-31342, rebased on current main. |
9b94ab2 to
d3339cd
Compare
janlindstrom
left a comment
janlindstrom
left a comment
There was a problem hiding this comment.
Please remove all references to MDEVs on source code you added. Code comments should be self readable without needing to read JIRA.
Build failure is because of rocsdb and not caused by these changes.
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for cleaning this up! LGTM. Please keep working with Jan on the final review.
d3339cd to
1d166ae
Compare
…able writes For simple single-schema SELECT queries on information_schema.tables, bypass ha_write_tmp_row() and stream rows directly via the protocol. Add EXPLAIN is_fast_path visibility and MTR coverage, including Windows EXPLAIN casing normalization.
…EMA.TABLES Stop the I_S.TABLES fast path after a matching row is sent, not after the first scanned table. Honour LIMIT / @@sql_select_limit via plan->max_rows so filtered rows do not count.
Set limit_found_rows from matching rows sent on the fast path so FOUND_ROWS() matches the fallback path when SQL_CALC_FOUND_ROWS is not used.
Keep the I_S.TABLES fast path only for plain client SELECTs. Fall back when the result must go through a file, ANALYZE, SQL_BUFFER_RESULT, PROCEDURE ANALYSE(), or the cursor protocol.
Add MTR coverage that ANALYZE SELECT uses the fallback path and does not assert or send a double EOF.
Add MTR coverage that SQL_BUFFER_RESULT uses the fallback path and returns the actual column values.
Add MTR coverage that PROCEDURE ANALYSE() is applied instead of being silently ignored on the fast path.
Document that the I_S.TABLES fast path is disabled for cursor protocol so MTR --cursor-protocol does not report ER_UNKNOWN_ERROR.
Do not use the fast path when the query has a subquery. Evaluating that predicate while get_all_tables() uses a temporary MEM_ROOT freed the subquery JOIN before outer JOIN cleanup.
Do not use the fast path when the statement uses a stored routine. Evaluating that function during fill changes THD open-table state and trips restore_backup_open_tables_state().
1d166ae to
61b6ee0
Compare
3 participants
MDEV-31342 Fix I_S fast-path follow-up bugs
https://jira.mariadb.org/browse/MDEV-31342
The INFORMATION_SCHEMA.TABLES fast path streamed rows through the client protocol for statements that still need the normal result sink. That broke ANALYZE, SELECT INTO OUTFILE, SQL_BUFFER_RESULT, PROCEDURE ANALYSE(), and cursor-protocol execution.
LIMIT 1 also stopped after the first scanned table rather than the first matching row. sql_select_limit and FOUND_ROWS() were ignored.
Keep the fast path only for plain client SELECTs. Count matching rows so LIMIT, sql_select_limit, and FOUND_ROWS() are correct.
Related:
Test plan