Implement support for aliasing

[weiznich]

This is currently WIP and should only be seen as demo and a request for comments
This PR takes another try on implement aliasing support.

Example

As first point just a short example how the API would look like:

    let user_alias = alias!(users as user_alias);

    posts::table
        .inner_join(user_alias)
        .inner_join(users::table)
        .select((user_alias.field(users::name), users::name))
        .load::<(String, String)>(&connection)
        .unwrap();

Feature

The current implementation allows:

• to define aliases
• to access columns of a defined alias (maybe we should also implement the API proposed by sean in the other PR here?)
• to use multiple aliases of the same/different tables to write subqueries
• to join aliases of the same/different table using explicit and implicit joins

Implementation

One of our problems with providing support for aliasing was that we were not sure how a good type safe API would look like. Sean already opened #1773 which would provide a type safe API, that requires more steps to setup an alias then we would like to have. Another idea floating around was to have an alias type in diesel itself, that would just wrap tables (and maybe later also other things), which would allow us to provide a much concrete API that wouldn't require that much steps to setup. We thought this was blocked on specialization because we would need to implement AppearsInFromClause for every combination and alias. In detail we need to implement that trait in such a way that the associated type Count is Never for all combination besides table and alias type are the same on both sides. It seemed impossible to solve that without having some kind of specialization. This PR does also not really solve this problem, it just takes another way to implement all combinations of AppearsInFromClause:

• It reuses the allow_tables_in_same_query! macro by extending the generated code there to also contain cases for aliased tables. This allows to mix aliases of different tables with other tables as long as you can use the tables together. This requires the recently stabilized re-rebalance-coherence feature, that is currently in the rust beta channel.
• Unfortunately it is not possible to implement AppearsInFromClause there in such a way that would allow to use two aliases of different tables together, because of rusts coherence rules (all types are not fundamental and not from the current crate). This is solved by the AliasNotEqualHelper trait which just moves this job to something (a table) for which we could implement traits there and a generic implementation of AppearsInFormClause for all alias alias combination that internally uses the concrete implementations of AliasNotEqualHelper
• We need to provide a implementation of AppearsInFromClause for the alias with itself. This again uses the AliasNotEqualHelper, but now inside the alias macro. This prevents that a alias is used more than once in a given query.
• As last open point we need to find some way to allow multiple aliases for the same table to be used in the same query. This requires another implementation of AppearsInFromClause for cases where two aliases share the same table, but are different. This impl would conflict again with already implemented ones, so I choose a hack to workaround this one. By declaring multiple aliases together in a single alias! macro call we are able to brute force this impls by reusing AliasNotEqualHelper again.

Anything else in this PR is basically just implementing already existing stuff also for aliases to write some simple test cases. The tests just check if the query compiles and is successfully executed.

Open questions

• Which features did I miss?
• Did I miss an important usecase?
• Are there any unnecessary restrictions in this API?
• Is this API sound?
• To we even want to have a API in this shape?

Things needed to be fixed for a final PR

• Correctly implement query dsl support for aliases (it requires currently that you call as_query() explicitly in most cases)
• revisit code organisation (suggestions welcome)
• discuss naming
• documentation
• more and better tests

[dessalines]

Assuming these conflicts are solved, is this working? There hasn't been movement on this in a year but if it works I'd love to use it.

[weiznich]

As there seems to be some interest in getting this feature somehow into diesel, I will try to writeup what I expect from a complete implementation. To start of with something: This PR is merely a demonstration of the underlying concept extracted from some of my own personal projects and somehow integrated into diesel.
To finish this implementation at least the following things are required:

• Rebasing this PR to the current diesel master branch
• Correctly implement all QueryDsl traits that make sense for aliases (That's already started with at least some of the traits)
• Review the test cases + maybe add more
• Review the compile tests + maybe add more
• Review the code structure + maybe move things into the right place
• Write quite a lot of documentation how to use this feature
• Have a look about other cases where aliases can occur, like expression aliases and aliases for select statements. Before merging this we should at least have and idea if it's possible to support such cases with the same/a similar implementation or if that's impossible. This likely will require quite a lot of experimentation.
• Have a look about how to write types (for example as return types) for queries containing an alias

So if anyone is interested feel free to take this PR as basis to work on. If you need some pointers feel free to ping me in our gitter channel.

cc @dessalines

[Ten0]

I have time to work on this, as this is something we often need. :)

I believe this is the correct design :)

I've updated this to the current master (or almost) at https://github.com/Ten0/diesel/tree/feature/aliasing (tests pass again).

If you can allow edits:

I can probably push that work here :)

[weiznich]

@Ten0 Thanks for working on this 👍 Edits are already allowed 🙈

[Ten0]

Edits are already allowed 🙈

Ah, then it looks like my permissions don't allow me to do this. (Maybe I have merge but not push or something)

Should I open a separate draft then?

[weiznich]

@Ten0 I've added you as Collaborator to my diesel fork, that should allow you to push to this branch.

[Ten0]

So I've tried some changes and I'm pretty happy with them:

• Instead of having Alias<T, F> where T is the table and F represents the name (which requires two generic parameters in most places and allows the incorrect representation Alias<T2, F1>), turn that into a single generic parameter that implements an AliasSource trait that provides both the table and the alias name. This makes aliases types easier to express.
• Reduce the amount of code generated in macros:
  • allow_tables_to_appear_in_same_query! now implements a single new trait: TableNotEqual<T2> for T1 for all pairs of distinct tables. This impl is then used within the diesel crate to generate the required implementations of AppearsInFromClause for both tables and aliases when tables are distinct. Implementations where tables are not distinct are still implemented by the table! macro (otherwise would conflict with previous impl because a downstream crate could impl TableNotEqual<T> for T), but that is O(n) generated code instead of O(n²) generated code.
  • Alias joins are implemented mostly within Diesel (rest generically by the table! macro). This is done by extending FieldAliasMapper so that it can convert a lot more expressions than just directly tables, leaving those that are from other tables untouched. This allows us to convert the join on clauses to their aliased counterparts in generic code.
    (Although pretty minor, this will also enable the user to convert several fields at a time in composite structures, e.g.:

    let posts = users::table.inner_join(posts::table.on(...).inner_join(users_alias))
    .select(users_alias.fields((posts::title, posts::body, users::username, users::last_seen)))
    .load::<Post>(db)?;

    )

[Ten0]

Have a look about other cases where aliases can occur, like expression aliases and aliases for select statements

I've experimented with this a bit (Ten0@107ae92) and found it would actually be pretty hard to support within the query builder. (Parentheses are not neutral, SelectableExpression !: Expression, we can't reference fields of an aliased subselect because their name is not typed, table does not impl QueryFragment)...

Given this I think it's reasonable to ignore this for now, and either:

• Have a separate structure to handle this later
• Have that in Diesel 3
• Decide at that time that people are probably not using complex trait bounds on Alias and pretend it wouldn't be a breaking change

[weiznich]

@Ten0 Thanks for the summary ❤️ . I will try to have a look hopefully later this week. If someone else from @diesel-rs/reviewers want to review this sooner feel free to do so.

[weiznich]

I did not finish the whole PR yet, but I rather like to leave the comments I've already made.
Thanks for the update 👍

[weiznich]

I would prefer something like:

const USER_ALIAS: Alias<user_alias> = diesel::alias!(schema::users as user_alias);

here if possible. Making the declared identifier visible, instead of hiding it behind a macro. That makes it easier to search for the corresponding definition as it looks like any other rust code.

[Ten0]

I don't think that would be possible because that would make this macro an expression macro so the user_alias type would be scoped to inside the construction of that expression at best.

My goal was to make it feel like table! on use (I had to use a const for that instead of a unit struct because of the alias wrapper).

[weiznich]

If we could not make it an expression maco, I would like to suggest the same solution as for sql_function! here. Just use diesel::alias!(const MY_ALIAS_IDENT: UserAlias = schema::users as UserAlias); (or something slightly different as long as it matches the normal rust const definition) to define an alias at module level.

[Ten0]

So the writing would then probably be:

//                    const name       AliasSource            alias name within the SQL query
diesel::alias!(const USERS_ALIAS: Alias<UsersAlias> = schema::users as users_alias);
USERS_ALIAS.select(...)

I'm not sure whether the fact it's a const instead of e.g. a unit struct that can be instantiated implicitly is typically relevant to the end user - or what they would focus on.
Maybe in our codebase we'd favor the table!-like writing to the fn-like writing when declaring these at the module level @Elrendio wdyt? (vs

diesel::alias!(schema::users as users_alias: UsersAlias);
users_alias.select(...)

)

[Ten0]

Although maybe it would be relevant considering if you're declaring at the module level that might be because you need to express the types so making the magic not magic would help with that.

[Ten0]

For now I'm supporting both (150ce50)

[Elrendio]

Hi 👋

After discussing it IRL with @Ten0 I'm in favor of diesel::alias!(schema::users as users_alias: UsersAlias); and even better diesel::alias!(schema::users as users_alias); when you don't need to reference the type.

Arguments:

1. I find diesel::alias!(schema::users as users_alias); simplier than diesel::alias!(const USERS_ALIAS: Alias<UsersAlias> = schema::users as users_alias); as there's less boiler plate. Seems to me it will be simplier for developers to adopt the feature
2. I don't think developpers need to know the internals of the feature before using it. Most people @Stockly for example will just want to be able to reference an alias in a query.
3. Having the same writing for when using an alias in a function and in a module feels simplier to me. Notably, it will enable to just move the line of code out of the function.
4. We can support both writings for people that want to deep dive a bit more

Have a nice day,
Elrendio

[Ten0]

TODO if we keep the const writing we probably want to use static_cond! to prevent from using:

diesel::alias! {
    const USERS_ALIAS: Alias<UsersAlias> = schema::users as users_alias;
    const USERS_ALIAS_2: Alias<UsersAlias2> = schema::users as users_alias;
}

in the same query

EDIT done in eac6342

[weiznich]

I was quite busy with other things the last two weeks. Hopefully I'm able to have a deeper look at this tomorrow.

[weiznich]

I've experimented with this a bit (Ten0/diesel@107ae92) and found it would actually be pretty hard to support within the query builder. (Parentheses are not neutral, SelectableExpression !: Expression

I've played a bit with this and got that here working: weiznich@2e23b87 This obviously needs some more work to figure out how to reference those aliases from somewhere else (group by, order, …) + to polish the interface.
That does not mean we need to support this in the initial version, just that we should name stuff so that this is possible (+ maybe add that QueryFragment impl for the generated table structs as this could be a potentially breaking change).

we can't reference fields of an aliased subselect because their name is not typed,

Give expression aliases we could enforce that users could only reference aliased expressions from the select clause of the inner alias query in the outer query. So something like this:

let max_user_id = alias!(max(users::id) as inner_user_id);
let user_query_alias = alias!(users::table.filter(…).select(user_id) as filtered_users);

posts::table.inner_join(user_query_alias).filter(max_user_id.gt(5))

Again that does not need to be implemented now, we just should look if there are no major blockers for this. (I know even that is not trivial…)

table does not impl QueryFragment)

As written above, that's the simplest thing to fix 🙈 (we should probably just do that, it think that's purely for historic reasons that table does not implement QueryFragement directly...)

[weiznich]

Looks good modulo those two small points 🎉

[weiznich]

One other thing I forgot to mention above: This misses a changelog entry 😉

[bitemyapp]

Pretty excited about this PR btw, table aliasing is one of the few things that forces me to write raw SQL queries. Thank you everyone for your hard work!

[Ten0]

I've experimented with this a bit (Ten0/diesel@107ae92) and found it would actually be pretty hard to support within the query builder. (Parentheses are not neutral, SelectableExpression !: Expression

I've played a bit with this and got that here working: weiznich@2e23b87 This obviously needs some more work to figure out how to reference those aliases from somewhere else (group by, order, …) + to polish the interface. That does not mean we need to support this in the initial version, just that we should name stuff so that this is possible (+ maybe add that QueryFragment impl for the generated table structs as this could be a potentially breaking change).

So the issue I had with this approach when getting to this point is the "Parentheses are not neutral, SelectableExpression !: Expression" issue, that is:

SELECT (("users"."name" || '!')) AS "name_alias" FROM "users";

is valid, but none of these are:

SELECT (("users"."name" || '!')) AS "name_alias" AS "name_alias_2" FROM "users";
SELECT ((("users"."name" || '!')) AS "name_alias") || '!' FROM "users";
SELECT "users"."name" FROM "users" WHERE "users"."name" AS "name_alias" = 'Sean';
-- or even
SELECT (("users"."name") AS "name_alias") FROM "users";

and with these trait bounds, all of these could be written.

(as branch: Ten0@36292a9)

So it looks like impl<S> Expression for Alias<S> is not really correct, and we would need to have a different trait for pseudo-expressions that can only be used at the top-level of a select statement. (I mistakenly confused this with SelectableExpression in my previous message but it looks like this trait currently doesn't exist).

In addition, methods such as .field()/.fields() don't really make sense... so it would feel more natural to me to have a different type for expression aliases anyway (esp. since it's located in query_source).

However I think there's a reasonably high chance that we'd use this for named subselects eventually, and I also think Table would not be appropriate there, so I've updated it to Target. 👍 (I think Source may be confusing since there's also AliasSource which is a different concept.)

[weiznich]

Looks ready now 👍
So lets move on merging this and see how it goes.
(That implies that I need to update my documentation/private item reform PR again. Hopefully we can land that one next)

[weiznich]

CI is falling due to to the already merged clap fixes. So let's move on and merge this one as well 🎉