CTE Implementations in Diesel

[smklein]

CTEs in Diesel

I'm going to start this discussion thread as a place to document some rough edges / ideas for implementing CTEs in Diesel.

This is a pretty raw space, so it's going to exist more as a "notepad" than a "proposal" of any sort. However, if you, the reader, have also implemented CTEs in Diesel, I'd be interested in hearing what worked well/poorly for you as well.

Background

I recently implemented a CTE - Common Table Expression - in Diesel.

https://github.com/smklein/tirefire/blob/35ca19da2a615ebef2a902ce93ba65a9879b70a7/src/lib.rs

This particular CTE implements the following SQL, though it could certainly be made more generic:

WITH found   AS (SELECT <primary key> FROM T WHERE <primary key = value>)                                                                                                                                                              
     updated AS (UPDATE T SET <constraints> RETURNING *)                                                                                                                                                                               
SELECT                                                                                                                                                                                                                                 
       found.<primary key> AS found_<primary_key>,                                                              
       updated.<primary key> AS updated_<primary_key>                                                                                                                                                                                  
FROM                                                                                                            
       found                                                                                                    
LEFT JOIN                                                                                                       
       updated                                                                                                  
ON                                                                                                              
       found.<primary_key> = updated.<primary_key>;   

What does this CTE do

Basically, it's an improvement on the return code of UPDATE statements. When making an UPDATE, the response from the DB will be either "some rows have been updated" or "no rows have been updated". However, there are cases where it can be useful to also identify "does the row exist" in the same statement.

As a concrete example, when implementing optimistic concurrency control (like "UPDATE id = $1 WHERE generation_number = $2") it is useful to know if the row exists at all, separately from the question "is the generation number what we expect it to be".

What worked well

• (EXAMPLES) The Diesel guide referenced a pagination example that was quite useful. More similar "extending Diesel" examples would help! ... also, if this CTE would be a useful example, I'd be happy to add it.
• (SUBQUERIES SURPRISINGLY DOABLE) The structure of QueryFragment makes it pretty easy to both create new subqueries and reference caller-supplied subqueries.
• (POSSIBLE TO EXTEND EXISTING STATEMENTS) I found it really nice that my syntax could extend the UpdateStatement, so the vast majority of existing code I'm using that already calls diesel::update can just import a new trait and get going.

What could have been better

• (GNARLY TRAIT BOUNDS) This is a little tricky to quantify - and possibly not the responsibility of this crate - but implementing a hand-rolled CTE requires a lot of trait bounds, which have not-the-best compiler errors. I saw the error error[E0275]: Overflow evaluting the requirement _ quite a lot, which suggests increasing the recursion_limit (wrong) instead of adding more bounds (right, but requires knowing what bounds to add, which can be tricky).
• (STRUCT VISIBILITY) As mentioned here, it's not possible to reference the "ReturningClause" bounds of UpdateStatements from outside Diesel - as of 1.4.7, those structures are private.
  Example:

// We want to use an update statement without a returning clause.
struct MyStatement<T, U, V> {
  // OK - Ret defaults to NoReturningClause
  update: UpdateStatement<T, U, V>, 
}
// We want to use an update statement with a specific return clause.
struct MyStatement<T, U, V> {
  // NOT OK - Private struct
  update: UpdateStatement<T, U, V, ReturningClause<_>>,
},

• (LOSS OF TYPES + DOT NOTATION KINDA GROSS) Within my CTE, I used AS statements to give names to generated rows. Referring to those rows requires accessing generated_row.field.
  Example:
  This creates "updated AS ([update statement] RETURNING *)":

        out.push_sql("), updated AS (");
        self.update_statement.walk_ast(out.reborrow())?;
        out.push_sql(" RETURNING *) ");

And this references the "updated" row, which we created from the output of that prior update statement:

        out.push_sql(", updated.");
        out.push_identifier(name)?;

This has some issues: (1) The type info from the update subquery is totally lost - the idea that the update statement returns all columns is not embedded into the type system, as it would be for typical Diesel update statements. (2) Referencing the newly generated row (updated.<primary key>, in this case) currently requires some manual string concatenation. If I could somehow access a type representing the output of self.update_statement, and identify that it gets aliased to updated, I would gladly reference columns which are made accessible in a type-safe manner.

As a totally made up, handwavey example:

        // This preserves the type information of the original update statement.
        // (Let's assume this has a `.returning` clause)
        let update_subquery = self.update_statement.as("updated");

        // Emits "updated AS (<update subquery>)"
        self.update_statement.walk_ast(out.reborrow())?;
       
        ...

        // Later, when referencing the output of the UPDATE subquery...
        // Emits "updated.id"
        out.push_identifier(<update_subquery::Id as Column>::NAME);

[weiznich]

First of all thanks for writing down this extensive overview of what worked well and what didn't work that well. I will try to address a few points below, so that we can start taking concrete actions here.

(EXAMPLES) The Diesel guide referenced a pagination example that was quite useful. More similar "extending Diesel" examples would help! ... also, if this CTE would be a useful example, I'd be happy to add it.

More examples and more complete guides are always welcome. The source for the webpage can be found here. Additionally it would be required to have the complete example in tree in our example folder, so that everything is tested and will not break in future versions. That written: We are currently in some wired transition period between diesel 1.0 and diesel 2.0 where some details about the implementation may change, so it might be required for the example to be added in the 1.4.x branch as well.

(GNARLY TRAIT BOUNDS) This is a little tricky to quantify - and possibly not the responsibility of this crate - but implementing a hand-rolled CTE requires a lot of trait bounds, which have not-the-best compiler errors. I saw the error error[E0275]: Overflow evaluting the requirement _ quite a lot, which suggests increasing the recursion_limit (wrong) instead of adding more bounds (right, but requires knowing what bounds to add, which can be tricky).

I would be interested in collecting concrete examples where this did occur. Sometimes they are fixable on diesels side and I remember that some of them have been fixed on the master branch already. It's likely a good idea to add such example to the compile-test suite so that we can track those errors and see if we can do better.
Other than that we can try to raise attention for such issues at the rustc repo. As far as I know they are always interested to hear about cases where the diagnostics are sub optimal and get suggestions how to do that better. To list some existing issues to reference:

rust-lang/rust#83383
rust-lang/rust#81374
rust-lang/rust#78982
rust-lang/rust#85159
rust-lang/rust#34260
rust-lang/rust#46309
rust-lang/rust#47032

If you raise a new issue at the rustc repo feel free to ping me from there.

Another side note related to that: Both #[rustc_on_unimplemented] and #[do_not_recommend] may improve the situation here if they hit stable at some point in time.

STRUCT VISIBILITY) As mentioned here, it's not possible to reference the "ReturningClause" bounds of UpdateStatements from outside Diesel - as of 1.4.7, those structures are private.

This is clearly a "bug" in diesel that should be fixed. As UpdateStatement and InsertStatement are already part of our public API I would propose that we just expose ReturningClause and NoReturningClause via the query_builder module. It just needs a PR that adds a reexport line here (with #[doc(inline)]) + documentation on both types here (Just write the documentation as doc comment above the corresponding type, the macro can handle that)

(LOSS OF TYPES + DOT NOTATION KINDA GROSS) Within my CTE, I used AS statements to give names to generated rows. Referring to those rows requires accessing generated_row.field.
Example:
This creates "updated AS ([update statement] RETURNING *)":

Having this feature would mean to have general aliasing support, which is definitively something we are interested in. That written: It's nothing we exactly know how to implement yet. I've done some experimentation with table aliasing in #2254 but as listed in my last comment there finishing that PR requires a lot of work. For me that's something that can wait till after the 2.0 release as this does not require breaking changes and the strategy outlined there also works as third party implementation. That written aliasing support is something that definitively needs to be there before we can start working on CTE support inside of diesel itself.