Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use postgres_backend feature flag for SupportsCombinationClause impl #3841

Merged

Conversation

hgzimmerman
Copy link
Contributor

@hgzimmerman hgzimmerman commented Oct 23, 2023

I found that I couldn't compile queries that contained CombineDSL::union and CombineDSL::union_all expressions with the postgres feature flag replaced with postgres_backend while switching to diesel-async.

I believe that this is a simple change to allow use of the CombineDsl trait with just the postgres_backend feature. In a quick search along with personal experience from making the feature-flag switch in my project, I didn't come across other cases where postgres is used instead of postgres_backend as a feature-gate.

I found that I couldn't compile queries that contained union and union_all expressions with the `postgres` feature flag turned replaced with `postgres_backend` while switching to `diesel-async`.
I believe that this is a simple change to allow use of the `CombineDSL` trait with just the `postgres_backend` feature.
@weiznich weiznich requested a review from a team October 24, 2023 07:20
@Ten0
Copy link
Member

Ten0 commented Oct 24, 2023

I didn't come across other cases where postgres is used instead of postgres_backend as a feature-gate

I'm guessing the whole connection implementation based on libpq?

#[cfg(feature = "postgres")]
pub(crate) mod connection;

@hgzimmerman
Copy link
Contributor Author

hgzimmerman commented Oct 24, 2023

Sorry, I was imprecise in my wording there. Aside from the contents of pg::connection, I didn't come across any other feature-flagged modules that impact Diesel's DSL with respect to postgres. The feature flag for "postgres" on the connection module is appropriate.

My intent was to remove my dependency on libpq by only using the implementation in tokio-postgres depended upon by diesel-async, but ran into this limitation.

@weiznich weiznich added this pull request to the merge queue Oct 24, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2023
@weiznich weiznich added this pull request to the merge queue Oct 24, 2023
Merged via the queue into diesel-rs:master with commit 89285c5 Oct 24, 2023
32 checks passed
@hgzimmerman hgzimmerman deleted the bug/postgres_backend_combine_dsl branch October 26, 2023 12:44
@weiznich weiznich added the maybe backport Maybe backport this pr to the latest diesel release label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maybe backport Maybe backport this pr to the latest diesel release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants