remove transactions in queries #421
Conversation
| isolation_level, | ||
| sql::ast::transaction::TransactionMode::ReadOnly, | ||
| ), | ||
| pre: vec![], |
There was a problem hiding this comment.
No more begin and commit for queries.
| TableAliasIndex(index) | ||
| /// Fetch the tracked native queries used in the query plan and their table alias, | ||
| /// and the global table index. | ||
| pub fn get_native_queries_and_global_index(self) -> (Vec<NativeQueryInfo>, TableAliasIndex) { |
There was a problem hiding this comment.
We break down the state so we can consume the native queries when processing native queries, and use the global table alias index outside of it when inventing names for the wrapping ctes.
| @@ -257,7 +257,8 @@ fn translate_native_query( | |||
|
|
|||
| // add the procedure native query definition is a with clause. | |||
| select.with = sql::ast::With { | |||
| common_table_expressions: crate::translation::query::native_queries::translate(env, state)?, | |||
| common_table_expressions: crate::translation::query::native_queries::translate(env, state)? | |||
There was a problem hiding this comment.
mutations do need to run native queries at the top-level, because they are mutations. So we don't wrap the ctes here.
| common_table_expressions: { | ||
| let (ctes, mut global_table_index) = native_queries::translate(&env, state)?; | ||
| // wrap ctes in another cte to guard against mutations in queries | ||
| ctes.into_iter() | ||
| .map(|cte| native_queries::wrap_cte_in_cte(&mut global_table_index, cte)) | ||
| .collect() | ||
| }, |
| @@ -72,5 +75,38 @@ pub fn translate(env: &Env, state: State) -> Result<Vec<sql::ast::CommonTableExp | |||
| }); | |||
| } | |||
|
|
|||
| Ok(ctes) | |||
| Ok((ctes, global_table_index)) | |||
There was a problem hiding this comment.
We return the global table index here as well so it can be used when wrapping ctes, but the native queries with the state is consumed.
| /// Wrap a CTE in another CTE so we can guard against mutations in queries. | ||
| pub fn wrap_cte_in_cte( | ||
| table_alias_index: &mut TableAliasIndex, | ||
| mut cte: sql::ast::CommonTableExpression, | ||
| ) -> sql::ast::CommonTableExpression { | ||
| // This is the name the rest of the query knows, so we keep it on the outer parts. | ||
| let outer_cte_alias = cte.alias; | ||
|
|
||
| // this is going to be internal, so we replace the CTE. | ||
| cte.alias = table_alias_index.make_table_alias(outer_cte_alias.name.clone()); | ||
|
|
||
| // build the internal `WITH <CTE> SELECT * FROM CTE as <nested_cte_alias>`. | ||
| let nested_cte_alias = table_alias_index.make_table_alias(outer_cte_alias.name.clone()); | ||
|
|
||
| let nested_cte_select = sql::ast::CTExpr::Select({ | ||
| let mut select = sql::helpers::star_select(sql::ast::From::Table { | ||
| reference: sql::ast::TableReference::AliasedTable(cte.alias.clone()), | ||
| alias: nested_cte_alias.clone(), | ||
| }); | ||
| select.with = sql::ast::With { | ||
| common_table_expressions: vec![cte], | ||
| }; | ||
| select | ||
| }); | ||
|
|
||
| // wrap in another CTE. | ||
| sql::ast::CommonTableExpression { | ||
| alias: outer_cte_alias, | ||
| column_names: None, | ||
| select: nested_cte_select, | ||
| } |
| cast( | ||
| ( | ||
| ("%0_%variables_table"."%variables" -> $2) #>> cast(ARRAY [] as text[])) as int4),cast((("%0_%variables_table"."%variables" -> $3) #>> cast(ARRAY [] as text[])) as int4)) AS series) AS arr | ||
| WITH "%9_NATIVE_QUERY_array_series" AS ( |
There was a problem hiding this comment.
an example of a wrapping.
There was a problem hiding this comment.
begin and commit no more.
There was a problem hiding this comment.
Test that this works.
There was a problem hiding this comment.
This is what I wanted to see. Thank you!
There was a problem hiding this comment.
This is what I wanted to see. Thank you!
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
What
Running queries in transactions require that we run 3 statements: begin, the query, commit.
We use transactions to enforce a
READ ONLYsetting so that native queries which run mutations will be rejected.However, each roundtrip takes time. If we can avoid it, it would improve the performance of queries.
In the PR we use the fact the mutating sql statements can only occur in a CTE at the top-level, and wrap native queries in another layer of CTE, thereby enforce the read only requirement without using transactions.
How
beginandcommitstatements before/after a query.WITH <native_query_name> as (WITH <alias> as (<native_query>) SELECT * FROM CTE as <alias>) SELECT ...