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

Add merge to Arguments #3652

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dgagn
Copy link

@dgagn dgagn commented Dec 22, 2024

Add merge to Arguments

Currently, there is no convenient way to merge two sets of query builder arguments.

Why?

In my use case, I need to combine arguments from multiple queries, but there's no existing method to do this.

Here is one of my use case

I am building a custom QueryBuilder that would like to take Arguments as a parameter, but can't because of the limitation of the add method on the trait.

I would like something like :

macro_rules! binding {
    [$($arg:expr),* $(,)?] => {{
        let mut args = <Postgres as sqlx::Database>::Arguments::default();
        $(
            args.add($arg).expect("failed to add binding");
        )*
        args
    }};
}

CustomBuilder::new()
    .select_raw("price * ? as price_tax, ? as name", binding![1.2, "test"])
    .from("products")
    .where_raw("price > ?", binding![10])
    // ...

fn select_raw_expr_bindings<T, B>(mut self, columns: T, bindings: B) -> Self
where
    T: Into<String>,
    B: Arguments<'q>,
{
    self.ensure_bindings();
    self.bindings
        .as_mut()
        .expect("bindings should be initialized")
        .merge(bindings); // Merge here
    self.columns = ColumnExpr::Raw(columns.into());
    self
}

But, this is what I have at the moment :

pub struct SelectBuilder<'q> {
    columns: ColumnExpr<'q>,
    table: Ident<'q>,
    bindings: Option<<Postgres as Database>::Arguments<'q>>,
    where_node: Option<Node<'q>>,
    // ...
}

// ...

fn select_raw_expr<T, B>(mut self, columns: T, bindings: B) -> Self
where
    T: Into<String>,
    B: IntoIterator,
    B::Item: 'q + sqlx::Encode<'q, Postgres> + sqlx::Type<Postgres>,
{
    self.ensure_bindings();
    for binding in bindings {
        self.bindings
          .as_mut()
          .expect("bindings should be initialized")
          .add(value)
          .expect("failed to add value to bindings");
    }
    self.columns = ColumnExpr::Raw(columns.into());
    self
}

What I have has a significant limitation; it requires all bindings in the iterator to be of the same type. For example, [1, "test"] would not work because it combines different types (integer and str) in the same collection.

Introducing a merge function would allow developers to combine multiple sets of Arguments.

I also have other use cases in subqueries, where raw and so on.

Thank you

@dgagn dgagn changed the title Merge arguments Add merge to Arguments Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant