MariaDB - avoid using correlated sub-queries #14630
Merged
+170
−72
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…QL, it needs to wrap the query the same as all other DBs, however it needs to apply the where statement in a slightly different manner.
…her than using a limited sub-query which is dis-allowed in MySQL/MariaDB due to the nature of the correlated sub-query.
samwho
reviewed
Sep 24, 2024
…dibase into fix/mysql-correlated-queries
… problem, this solves for it by separating them and allowing us to use the special json_arrayagg for mariaDB, but use a correlated sub-query for MySQL.
samwho
approved these changes
Sep 24, 2024
samwho
reviewed
Sep 24, 2024
| @@ -210,16 +210,27 @@ function buildDeleteTable(knex: SchemaBuilder, table: Table): SchemaBuilder { | |||
|
|
|||
| class SqlTableQueryBuilder { | |||
| private readonly sqlClient: SqlClient | |||
| private extendedSqlClient: SqlClient | undefined | |||
There was a problem hiding this comment.
Ooh how come this is necessary? Why can't we just use sqlClient?
There was a problem hiding this comment.
We need to know that the implementation/dialect is MY_SQL - but also its MariaDB - for Knex it needs to know its MySQL, technically we could get around this by setting everywhere that needs to know its MySQL based on it being MariaDB when needed, but I like the idea of separating the difference in client here, since its still a MySQL datasource, it just reads a different version once it connects.
mike12345567
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a problem that was raised by our QA MariaDB database which was hitting a limit around how much data can be returned from an aggregate response - the JSON that was being returned was malformed and not performing as expected.
The issue was that we did not apply a limit to MySQL/MariaDB queries as it wasn't possible to limit these without the use of a correlated sub-query, something which is not allowed - information about this here.
To get around this we have used a limit which can be applied within the aggregation itself, avoiding the malformed response. This is not as performant as using a correlated sub-query, but we were very limited in terms of options and this gave us the best set of fixed problems while performing within acceptable limits.