ronantakizawa/codereview-bench huggingface dataset:
Repository: diesel-rs/diesel
File: diesel/src/sqlite/expression/functions.rs (Rust)

Code before:
    ///     return Ok(());
    /// }
    /// let result = diesel::select(json_quote::<Integer, _>(42))
    ///     .get_result::<Option<String>>(connection)?;
    /// assert_eq!(Some("42".to_string()), result);
    ///
    /// let result = diesel::select(json_quote::<Text, _>("verdant"))
    ///     .get_result::<Option<String>>(connection)?;
    /// assert_eq!(Some("\"verdant\"".to_string()), result);
    ///
    /// let result = diesel::select(json_quote::<Text, _>("[1]"))
    ///     .get_result::<Option<String>>(connection)?;
    /// assert_eq!(Some("\"[1]\"".to_string()), result);
    ///
    /// let result = diesel::select(json_quote::<Nullable<Text>, _>(None::<&str>))
    ///     .get_result::<Option<String>>(connection)?;
    /// assert_eq!(Some("null".to_string()), result);
    ///
    // COMENT----------------------------------------
    // BELOW NONE WORKING TESTS, JSON and FLOAT TYPES
    // DATATYPE FOR FLOAT NEEDED & JSON IS TREATED AS TEXT
    // let result = diesel::select(json_quote::<Json, _>(json!([1])))
    //     .get_result::<Option<String>>(connection)?;
    // assert_eq!(Some("[1]".to_string()), result);
    // 
    // let result = diesel::select(json_quote::<Integer, _>(3.14159))
    //     .get_result::<Option<String>>(connection)?;
    //
    // assert_eq!(Some("3.14159".to_string()), result);
    // COMENT----------------------------------------
    ///
    /// # Ok(())
    /// # }
    /// ```
    #[sql_name = "json_quote"]
    #[cfg(feature = "sqlite")]
    fn json_quote<J: MaybeNullableValue<Json> + MaybeNullableValue<Integer> + MaybeNullableValue<Text>>(  // Have to add Supoprt for Real/Floates, how? Existing Datatype?
        j: J
    ) -> Nullable<Text>;
}


Review comment (suggestion):
```suggestion
    // let result = diesel::select(json_quote::<Float, _>(3.14159))
```

Code after:
    ///     return Ok(());
    /// }
    /// let result = diesel::select(json_quote::<Integer, _>(42))
    ///     .get_result::<Value>(connection)?;
    /// assert_eq!(json!(42), result);
    ///
    /// let result = diesel::select(json_quote::<Text, _>("verdant"))
    ///     .get_result::<Value>(connection)?;
    /// assert_eq!(json!("verdant"), result);
    ///
    /// let result = diesel::select(json_quote::<Text, _>("[1]"))
    ///     .get_result::<Value>(connection)?;
    /// assert_eq!(json!("[1]"), result);
    ///
    /// let result = diesel::select(json_quote::<Nullable<Text>, _>(None::<&str>))
    ///     .get_result::<Value>(connection)?;
    /// assert_eq!(json!(null), result);
    ///
    /// let result = diesel::select(json_quote::<Double, _>(3.14159))
    ///     .get_result::<Value>(connection)?;
    /// assert_eq!(json!(3.14159), result);
    ///
    /// let result = diesel::select(json_quote::<Json, _>(json!([1])))
    ///     .get_result::<Value>(connection)?;
    // assert_eq!(json!([1]), result);
    ///
    ///
    /// # Ok(())
    /// # }
    /// ```
    #[sql_name = "json_quote"]
    #[cfg(feature = "sqlite")]
    fn json_quote<J: SqlType + SingleValue>(j: J) -> Json;
}
Repository: diesel-rs/diesel
File: diesel/src/expression/cast.rs (Rust)

Code before:
        $(
            $(
				#[cfg(feature = $backend_feature)]
                $(#[cfg(feature = $feature)])?
                impl KnownCastSqlTypeName<$backend> for sql_types::$type {
                    const SQL_TYPE_NAME: &'static str = $val;
                }
            )*
        )*
    };
}
type_name! {
    diesel::pg::Pg: "postgres_backend" {
        Bool => "bool",
        Int2 => "int2",
        Int4 => "int4",
        Int8 => "int8",
        Float => "float4",
        Double => "float8",
        Numeric => "numeric",
        Text => "text",
        Date => "date",
        Interval => "interval",
        Time => "time",
        Timestamp => "timestamp",
        "uuid"; Uuid => "uuid",
        Json => "json",
        Jsonb => "jsonb",
    }
    diesel::mysql::Mysql: "mysql_backend" {
        Int4 => "integer",
        Int8 => "integer",
        Text => "char",

    }
    diesel::sqlite::Sqlite: "sqlite" {
        Int4 => "integer",
        Int8 => "bigint",
        Text => "text",
    }
}

impl<S, E, ST> FieldAliasMapper<S> for Cast<E, ST>
where
    S: AliasSource,
    E: FieldAliasMapper<S>,
{
    type Out = Cast<<E as FieldAliasMapper<S>>::Out, ST>;

    fn map(self, alias: &query_source::Alias<S>) -> Self::Out {
        Cast {

Review comment (suggestion):
Is the Uuid **sql** type not defined unconditionally on whether the uuid feature is enabled? (If yes then cfg directives below should probably also be changed to only check for `postgres_backend`) 

Code after:
        $(
            $(
				#[cfg(feature = $backend_feature)]
                impl KnownCastSqlTypeName<$backend> for sql_types::$type {
                    const SQL_TYPE_NAME: &'static str = $val;
                }
            )*
        )*
    };
}

type_name! {
    diesel::pg::Pg: "postgres_backend" {
        Bool => "bool",
        Int2 => "int2",
        Int4 => "int4",
        Int8 => "int8",
        Float => "float4",
        Double => "float8",
        Numeric => "numeric",
        Text => "text",
        Date => "date",
        Interval => "interval",
        Time => "time",
        Timestamp => "timestamp",
        Uuid => "uuid",
        Json => "json",
        Jsonb => "jsonb",
    }
    diesel::mysql::Mysql: "mysql_backend" {
        Int8 => "signed",
        Text => "char",
        Date => "date",
        Datetime => "datetime",
        Decimal => "decimal",
        Time => "time",
    }
    diesel::sqlite::Sqlite: "sqlite" {
        Int4 => "integer",
        Int8 => "bigint",
        Text => "text",
        Json => "json",
        Jsonb => "jsonb",
    }
}

impl<S, E, ST> FieldAliasMapper<S> for Cast<E, ST>
where
    S: AliasSource,
    E: FieldAliasMapper<S>,
{
Repository: diesel-rs/diesel
File: diesel/src/expression/cast.rs (Rust)

Code before:
}

/// Marker trait: this SQL type (`Self`) can be casted to the target SQL type, but some values can be invalid
pub trait FaillibleCastsTo<ST> {}

impl<ST1, ST2> FaillibleCastsTo<sql_types::Nullable<ST2>> for sql_types::Nullable<ST1> where
    ST1: CastsTo<ST2>
{
}

/// Marker trait: this SQL type (`Self`) can be casted to the target SQL type
/// (`ST`) using `CAST(expr AS target_sql_type)`
pub trait CastsTo<ST>: FaillibleCastsTo<ST> {}

impl<ST1, ST2> CastsTo<sql_types::Nullable<ST2>> for sql_types::Nullable<ST1> where ST1: CastsTo<ST2>
{}

macro_rules! casts_impl {
    (
        $(
            $($feature: literal : )? ($to: tt <- $from: tt),
        )+
    ) => {
        $(
            $(#[cfg(feature = $feature)])?
            impl FaillibleCastsTo<sql_types::$to> for sql_types::$from {}
            $(#[cfg(feature = $feature)])?
            impl CastsTo<sql_types::$to> for sql_types::$from {}
        )+
    };
}

casts_impl!(
    (Bool <- Int4),
    (Float4 <- Int4),
    (Float8 <- Float4),
    (Float8 <- Int4),
    (Float8 <- Int8),
    (Int8 <- Int4),
    (Int8 <- Float4),
    (Int8 <- Float8),
    (Int4 <- Bool),
    (Int4 <- Float4),
    (Text <- Bool),
    (Text <- Float4),
    (Text <- Float8),
    (Text <- Int4),
    (Text <- Int8),
    (Text <- Date),
    (Text <- Json),
    (Text <- Jsonb),

Review comment (bug):
I'm not completely sure if we really should `impl FaillibleCastsTo for T: Cast`... On one hand this may work better in e.g. generated code, on the other one it may lead to users writing `.faillible_cast` instead of just `.cast` despite `.cast` being possible, which would make the code less readable and more scary for no reason.

What are your thoughts on this?

Code after:
}

/// Marker trait: this SQL type (`Self`) can be cast to the target SQL type, but some values can be invalid
pub trait FallibleCastsTo<ST> {}

impl<ST1, ST2> FallibleCastsTo<sql_types::Nullable<ST2>> for sql_types::Nullable<ST1> where
    ST1: CastsTo<ST2>
{
}

/// Marker trait: this SQL type (`Self`) can be cast to the target SQL type
/// (`ST`) using `CAST(expr AS target_sql_type)`
pub trait CastsTo<ST>: FallibleCastsTo<ST> {}

impl<ST1, ST2> CastsTo<sql_types::Nullable<ST2>> for sql_types::Nullable<ST1> where ST1: CastsTo<ST2>
{}

macro_rules! casts_impl {
    (
        $(
            $($feature: literal : )? ($to: tt <- $from: tt),
        )+
    ) => {
        $(
            $(#[cfg(feature = $feature)])?
            impl FallibleCastsTo<sql_types::$to> for sql_types::$from {}
            $(#[cfg(feature = $feature)])?
            impl CastsTo<sql_types::$to> for sql_types::$from {}
        )+
    };
}

casts_impl!(
    (Bool <- Int4),
    (Float4 <- Int4),
    (Float4 <- Int8),
    (Float8 <- Float4),
    (Float8 <- Int4),
    (Float8 <- Int8),
    (Int8 <- Int4),
    (Int8 <- Float4),
    (Int8 <- Float8),
    (Int4 <- Bool),
    (Int4 <- Float4),
    (Text <- Bool),
    (Text <- Float4),
    (Text <- Float8),
    (Text <- Int4),
    (Text <- Int8),
    (Text <- Date),
    (Text <- Json),
Repository: diesel-rs/diesel
File: diesel/src/sqlite/expression/helper_types.rs (Rust)

Code before:
use crate::dsl::AsExpr;
use crate::expression::grouped::Grouped;

/// The return type of `lhs.is(rhs)`.
pub type Is<Lhs, Rhs> = Grouped<super::operators::Is<Lhs, AsExpr<Rhs, Lhs>>>;

/// The return type of `lhs.is_not(rhs)`.
pub type IsNot<Lhs, Rhs> = Grouped<super::operators::IsNot<Lhs, AsExpr<Rhs, Lhs>>>;

pub use super::functions::return_type_helpers::*;


Review comment (refactor):
```suggestion
#[doc(inline)]
pub use super::functions::return_type_helpers::*;
```

to make sure that the documentation for these items still exists. We also need to double check if it works as expected.

Code after:
use crate::dsl::AsExpr;
use crate::expression::grouped::Grouped;

/// The return type of `lhs.is(rhs)`.
pub type Is<Lhs, Rhs> = Grouped<super::operators::Is<Lhs, AsExpr<Rhs, Lhs>>>;

/// The return type of `lhs.is_not(rhs)`.
pub type IsNot<Lhs, Rhs> = Grouped<super::operators::IsNot<Lhs, AsExpr<Rhs, Lhs>>>;

#[doc(inline)]
pub use super::return_type_helpers::*;
Repository: diesel-rs/diesel
File: diesel/src/sqlite/expression/helper_types.rs (Rust)

Code before:
use crate::dsl::AsExpr;
use crate::expression::grouped::Grouped;

/// The return type of `lhs.is(rhs)`.
pub type Is<Lhs, Rhs> = Grouped<super::operators::Is<Lhs, AsExpr<Rhs, Lhs>>>;

/// The return type of `lhs.is_not(rhs)`.
pub type IsNot<Lhs, Rhs> = Grouped<super::operators::IsNot<Lhs, AsExpr<Rhs, Lhs>>>;

pub use super::functions::return_type_helpers::*;


Review comment (refactor):
Somehow that module shows up in the documentation: 

![image](https://github.com/user-attachments/assets/2f7b5fa9-dd71-4d65-b75c-91bb489f8cf8)

That's in the docs of `diesel::sqlite::expression::dsl`

(There is also a list of exported helper types below, so it mostly works but not 100%)

Code after:
use crate::dsl::AsExpr;
use crate::expression::grouped::Grouped;

/// The return type of `lhs.is(rhs)`.
pub type Is<Lhs, Rhs> = Grouped<super::operators::Is<Lhs, AsExpr<Rhs, Lhs>>>;

/// The return type of `lhs.is_not(rhs)`.
pub type IsNot<Lhs, Rhs> = Grouped<super::operators::IsNot<Lhs, AsExpr<Rhs, Lhs>>>;

#[doc(inline)]
pub use super::return_type_helpers::*;
Repository: diesel-rs/diesel
File: diesel/src/expression/cast.rs (Rust)

Code before:
        $(
            $($feature: literal : )? ($to: tt <- $from: tt),
        )+
    ) => {
        $(
            $(#[cfg(feature = $feature)])?
            impl FallibleCastsTo<sql_types::$to> for sql_types::$from {}
            $(#[cfg(feature = $feature)])?
            impl CastsTo<sql_types::$to> for sql_types::$from {}
        )+
    };
}

casts_impl!(
    (Bool <- Int4),
    (Float4 <- Int4),
    (Float4 <- Int8),
    (Float8 <- Float4),
    (Float8 <- Int4),
    (Float8 <- Int8),
    (Int8 <- Int4),
    (Int8 <- Float4),
    (Int8 <- Float8),
    (Int4 <- Bool),
    (Int4 <- Float4),
    (Text <- Bool),
    (Text <- Float4),
    (Text <- Float8),
    (Text <- Int4),
    (Text <- Int8),
    (Text <- Date),
    (Text <- Json),
    (Text <- Jsonb),
    (Text <- Time),
    (Json <- Jsonb),
    (Jsonb <- Json),
    "mysql_backend": (Text <- Datetime),
    "postgres_backend": (Text <- Uuid),
);

macro_rules! fallible_casts_impl {
    (
        $(
            $($feature: literal : )? ($to: tt <- $from: tt),
        )+
    ) => {
        $(
            $(#[cfg(feature = $feature)])?
            impl FallibleCastsTo<sql_types::$to> for sql_types::$from {}
        )+
    };

Review comment (suggestion):
I removed the `Decimal <- Bool` cast from this list as that's not supported by postgres



Code after:
        $(
            $($feature: literal : )? ($to: tt <- $from: tt),
        )+
    ) => {
        $(
            $(#[cfg(feature = $feature)])?
            impl FallibleCastsTo<sql_types::$to> for sql_types::$from {}
            $(#[cfg(feature = $feature)])?
            impl CastsTo<sql_types::$to> for sql_types::$from {}
        )+
    };
}

casts_impl!(
    (Bool <- Int4),
    (Int4 <- Bool),
    (Int4 <- Float4),
    (Int8 <- Int4),
    (Int8 <- Float4),
    (Int8 <- Float8),
    (Float4 <- Int4),
    (Float4 <- Int8),
    (Float8 <- Float4),
    (Float8 <- Int4),
    (Float8 <- Int8),
    (Decimal <- Int4),
    (Decimal <- Int8),
    (Decimal <- Float4),
    (Decimal <- Float8),
    (Text <- Bool),
    (Text <- Float4),
    (Text <- Float8),
    (Text <- Int4),
    (Text <- Int8),
    (Text <- Date),
    (Text <- Json),
    (Text <- Jsonb),
    (Text <- Time),
    (Json <- Jsonb),
    (Jsonb <- Json),
    "mysql_backend": (Text <- Datetime),
    "postgres_backend": (Text <- Uuid),

);

macro_rules! fallible_casts_impl {
    (
        $(
            $($feature: literal : )? ($to: tt <- $from: tt),
        )+
    ) => {
Repository: diesel-rs/diesel
File: diesel_derives/src/tests/as_changeset.rs (Rust)

Code before:
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_1",
    );
}

#[test]
pub(crate) fn as_changeset_treat_none_as_null_1() {
    let input = quote::quote! {
        #[diesel(treat_none_as_null = true)]
        struct User {
            id: i32,
            name: Option<String>
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_treat_none_as_null_1",
    );
}

#[test]
pub(crate) fn as_changeset_table_name_1() {
    let input = quote::quote! {
        #[diesel(table_name = crate::schema::users)]
        struct User {
            id: i32,
            name: String
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_table_name_1",
    );
}

#[test]
pub(crate) fn as_changeset_primary_key_1() {
    let input = quote::quote! {
        #[diesel(primary_key(id, short_code))]
        struct User {
            id: i32,
            short_code: String,
            name: String
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,

Review comment (suggestion):
```suggestion
        #[diesel(table_name = crate::schema::some_other_user_table)]
```

Could we use a different table name than the "default" `users` here to demonstrate what's changed by this option?

(That requires updating the snapshots)

Code after:
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_1",
    );
}

#[test]
pub(crate) fn as_changeset_treat_none_as_null_1() {
    let input = quote::quote! {
        #[diesel(treat_none_as_null = true)]
        struct User {
            id: i32,
            name: Option<String>
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_treat_none_as_null_1",
    );
}

#[test]
pub(crate) fn as_changeset_table_name_1() {
    let input = quote::quote! {
        #[diesel(table_name = crate::schema::admin_users)]
        struct User {
            id: i32,
            name: String
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_table_name_1",
    );
}

#[test]
pub(crate) fn as_changeset_primary_key_1() {
    let input = quote::quote! {
        #[diesel(primary_key(id, short_code))]
        struct User {
            id: i32,
            short_code: String,
            name: String
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
Repository: diesel-rs/diesel
File: diesel_derives/src/tests/as_changeset.rs (Rust)

Code before:
    );
}

#[test]
pub(crate) fn as_changeset_embed_1() {
    let input = quote::quote! {
        struct User {
            id: i32,
            name: String,
            #[diesel(embed)]
            post: Post
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_embed_1",
    );
}

#[test]
pub(crate) fn as_changeset_change_field_to_type_1() {
    let input = quote::quote! {
        struct User {
            #[serialize_as = String]
            id: i32,
            name: String
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_change_field_to_type_1",
    );
}

#[test]
pub(crate) fn as_changeset_treat_none_field_as_null_1() {
    let input = quote::quote! {
        struct User {
            id: i32,
            #[diesel(treat_none_as_null = true)]
            name: Option<String>
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),

Review comment (suggestion):
As the `id` field will be skipped, this needs to be moved to a different field, otherwise we won't see any change in the output.

Also this needs to be `#[diesel(serialize_as = String)]`.

Code after:
    );
}

#[test]
pub(crate) fn as_changeset_embed_1() {
    let input = quote::quote! {
        struct User {
            id: i32,
            name: String,
            #[diesel(embed)]
            post: Post
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_embed_1",
    );
}

#[test]
pub(crate) fn as_changeset_change_field_type_1() {
    let input = quote::quote! {
        struct User {
            id: i32,
            name: String,
            #[diesel(serialize_as = String)]
            age: i32
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
        derive(syn::parse_quote!(#[derive(AsChangeset)])),
        "as_changeset_change_field_type_1",
    );
}

#[test]
pub(crate) fn as_changeset_treat_none_field_as_null_1() {
    let input = quote::quote! {
        struct User {
            id: i32,
            #[diesel(treat_none_as_null = true)]
            name: Option<String>
        }
    };
    expand_with(
        &crate::derive_as_changeset_inner as &dyn Fn(_) -> _,
        input,
Repository: diesel-rs/diesel
File: diesel_tests/tests/allow_tables_to_appear_in_same_query.rs (Rust)

Code before:
        id -> Integer,
    }
}

mod sub_table {
    use diesel::table;
    table! {
        table_b (id) {
            id -> Integer,
        }
    }
}

table! {
    table_c (id) {
        id -> Integer,
    }
}

allow_tables_to_appear_in_same_query!(table_a, table_c);
allow_tables_to_appear_in_same_query!(table_a, sub_table::table_b);

#[test]
fn test_allow_tables_to_appear_in_same_query_compiles() {
    // This test just needs to compile
}


Review comment (suggestion):
```suggestion

```

It's fine to just say that as top level comment

Code after:
//! Test submodule to verify that the `allow_tables_to_appear_in_same_query!` macro
//! can accept as input tables defined in the root module as well as in submodules.
use diesel::allow_tables_to_appear_in_same_query;
use diesel::table;

table! {
    table_a (id) {
        id -> Integer,
    }
}

mod sub_table {
    use diesel::table;
    table! {
        table_b (id) {
            id -> Integer,
        }
    }
}

table! {
    table_c (id) {
        id -> Integer,
    }
}

allow_tables_to_appear_in_same_query!(table_a, table_c);
allow_tables_to_appear_in_same_query!(table_a, sub_table::table_b);
Repository: diesel-rs/diesel
File: diesel_dynamic_schema/src/dynamic_select.rs (Rust)

Code before:
{
    fn walk_ast<'b>(&'b self, mut pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
        let mut first = true;
        for s in &self.selects {
            if first {
                first = false;
            } else {
                pass.push_sql(", ");
            }
            s.walk_ast(pass.reborrow())?;
        }
        Ok(())
    }
}

impl<DB, QS> ValidGrouping<()> for DynamicSelectClause<'_, DB, QS> {
    type IsAggregate = is_aggregate::No;
}

impl<'a, DB, QS> AsRef<[Box<dyn QueryFragment<DB> + Send + 'a>]>
    for DynamicSelectClause<'a, DB, QS>
{
    fn as_ref(&self) -> &[Box<dyn QueryFragment<DB> + Send + 'a>] {
        &self.selects
    }
}

impl<'a, DB, QS, F> FromIterator<F> for DynamicSelectClause<'a, DB, QS>
where
    F: QueryFragment<DB> + SelectableExpression<QS> + NonAggregate + Send + 'a,
    DB: Backend,
{
    fn from_iter<I: IntoIterator<Item = F>>(iter: I) -> Self {
        let mut select_clause = DynamicSelectClause::new();
        select_clause.add_fields(iter);
        select_clause
    }
}

impl<'a, DB, QS, F> std::iter::Extend<F> for DynamicSelectClause<'a, DB, QS>
where
    F: QueryFragment<DB> + SelectableExpression<QS> + NonAggregate + Send + 'a,
    DB: Backend,
{
    fn extend<I: IntoIterator<Item = F>>(&mut self, iter: I) {
        self.add_fields(iter)
    }
}

impl<'a, DB, QS> IntoIterator for DynamicSelectClause<'a, DB, QS> {
    type Item = Box<dyn QueryFragment<DB> + Send + 'a>;

Review comment (suggestion):
I would like to avoid exposing this internal representation as it makes harder to change it later, e.g adding more trait bounds to the trait object or similar changes.

Code after:
{
}

impl<QS, DB> Expression for DynamicSelectClause<'_, DB, QS> {
    type SqlType = Untyped;
}

impl<DB, QS> QueryFragment<DB> for DynamicSelectClause<'_, DB, QS>
where
    DB: Backend,
{
    fn walk_ast<'b>(&'b self, mut pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
        let mut first = true;
        for s in &self.selects {
            if first {
                first = false;
            } else {
                pass.push_sql(", ");
            }
            s.walk_ast(pass.reborrow())?;
        }
        Ok(())
    }
}

impl<DB, QS> ValidGrouping<()> for DynamicSelectClause<'_, DB, QS> {
    type IsAggregate = is_aggregate::No;
}

impl<'a, DB, QS, F> FromIterator<F> for DynamicSelectClause<'a, DB, QS>
where
    F: QueryFragment<DB> + SelectableExpression<QS> + NonAggregate + Send + 'a,
    DB: Backend,
{
    fn from_iter<I: IntoIterator<Item = F>>(iter: I) -> Self {
        let mut select_clause = DynamicSelectClause::new();
        select_clause.add_fields(iter);
        select_clause
    }
}

impl<'a, DB, QS, F> std::iter::Extend<F> for DynamicSelectClause<'a, DB, QS>
where
    F: QueryFragment<DB> + SelectableExpression<QS> + NonAggregate + Send + 'a,
    DB: Backend,
{
    fn extend<I: IntoIterator<Item = F>>(&mut self, iter: I) {
        self.add_fields(iter)
    }
}