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

Allow renaming fields for #[insertable_into] #300

Closed
sgrif opened this Issue Apr 23, 2016 · 3 comments

Comments

Projects
None yet
1 participant
@sgrif
Member

sgrif commented Apr 23, 2016

The column name should be able to differ from the field name. We already do this for tuple structs. We need to do this for named fields as well. I believe this already works, but we don't have an explicit test case for it. We need to make sure it's tested. I'm noting this as I'm reworking the entirety of how the derivation works, and we need to make sure this case is handled.

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 23, 2016

I think I'm going to rework the definition to allow pasting the struct def into the macro. We should test that all variants are handled.

  • Struct with pub
  • Struct without pub
  • Named fields with no annotations
  • Named fields with one or more #[column_name(name)]
  • Named fields with ignored attributes
  • Named fields with both #[column_name(name)] and ignored attributes
  • Named fields with pub and any of the above
  • Tuple fields with one or more #[column_name(name)]
  • Tuple fields with both #[column_name(name)] and ignored attributes
  • Tuple fields with pub and any of the above
  • Named or tuple fields which are optional with any of the above
@sgrif

This comment has been minimized.

Member

sgrif commented Apr 23, 2016

This is what the "bottom" of the macro will probably look like for Insertable:

    (
        table_name = $table_name:ident,
        struct_ty = $struct_ty:ty,
        lifetimes = ($($lifetime:tt),*),
        values_ty = $values_ty:ty,
        self_to_columns = $self_to_columns:pat,
        columns = ($($column_name:ident, $column_kind:ident),+),
    ) => { parse_as_item! {
        impl<$($lifetime,)* 'insert, DB> ::diesel::persistable::Insertable<$table_name::table, DB>
            for &'insert $struct_ty where
                DB: ::diesel::backend::Backend,
                $values_ty: ::diesel::persistable::InsertValues<DB>,
        {
            type Values = $values_ty;

            fn values(self) -> Self::Values {
                use ::diesel::expression::{AsExpression, Expression};
                use ::diesel::persistable::ColumnInsertValue;
                let $self_to_columns = *self;
                ($(
                    column_insert_expr!($table_name::$column_name, $column_name, $column_kind)
                ,)+)
            }
        }
    }};

Then it's just a matter of parsing all the various struct definitions into it. A messy and incomplete example for tuple structs:

    (
        ($table_name:ident)
        $(pub)* struct $struct_name:ident <$($lifetime:tt),+> ($(
            #[column_name($column_name:ident)]
            pub $field_type:ty,
        )+);
    ) => {
        Insertable! {
            table_name = $table_name,
            struct_ty = $struct_name<$($lifetime),+>,
            lifetimes = ($($lifetime),+),
            values_ty = ($(
                ::diesel::persistable::ColumnInsertValue<
                    $table_name::$column_name,
                    ::diesel::expression::bound::Bound<
                        <$table_name::$column_name as ::diesel::expression::Expression>::SqlType,
                        &'insert $field_type,
                    >,
                >
            ,)+),
            self_to_columns = $struct_name($(ref $column_name),+),
            columns = ($($column_name, regular),+),
        }
    };

Going to scrap most of the code from the stream this morning, as we need to fundamentally approach the macro differently, since the expression needs to focus on pattern matching not on field access. And I need to approach it from a parsing point of view as well. Will swing at this a bit more during tomorrows stream unless someone else wants to take over it.

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 24, 2016

This is addressed by #303

@sgrif sgrif closed this Apr 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment