SeaQL / sea-query

🔱 A dynamic SQL query builder for MySQL, Postgres and SQLite
https://www.sea-ql.org
Other
1.18k stars 195 forks source link

Alter `TableRef` for Any Statement Type That Implements a (New) Trait #445

Closed nahuakang closed 2 years ago

nahuakang commented 2 years ago

Motivation

Issue for self-assignment and tracking :)

Following this discussion, we need to implement a trait that allows the access and modification of TableRef on all statement types that implement the said trait to continue working on sea-orm #981. The statement types that should implement this trait includes:

Proposed Solutions

The trait:

pub trait SchemaDefinitionStatement {
    fn get_table_name(&self) -> Option<&TableRef>;

    fn set_table_name(&self) -> &mut Self;

    fn prefix_table_name_with_schema<A>(&mut self, schema: A) -> &mut Self
    where
        A: IntoIden + 'static;
}

Helper on TableRef:

impl TableRef {
    pub fn schema<A>(self, schema: A) -> Self
    where
        A: IntoIden + 'static,
    {
        match self {
            ...
        }
    }
}
nahuakang commented 2 years ago

@billy1624 I might try just add 1 method to SchemaDefinitionStatement since the type-related statements have TypeRef instead of TableRef and some statements have Vec<TableRef> instead of Option<TableRef>:

pub trait SchemaDefinitionStatement {
    fn prefix_table_name_with_schema<A>(&mut self, schema: A) -> &mut Self
    where
        A: IntoIden + 'static;
}

Let me know if you think it's a bad idea :)

billy1624 commented 2 years ago

@billy1624 I might try just add 1 method to SchemaDefinitionStatement since the type-related statements have TypeRef instead of TableRef and some statements have Vec<TableRef> instead of Option<TableRef>:

This make sense to me :P

But we might want to rename the trait then. Let me think... Thoughts? @nahuakang @ikrivosheev @tyt2y3

ikrivosheev commented 2 years ago

@nahuakang maybe add associated type to trait? For work with: TypeRef, TableRef?

nahuakang commented 2 years ago

@billy1624 also made this PR, which might resolve this issue?

billy1624 commented 2 years ago

Hey @nahuakang, you're correct! This feature is no longer needed. This is part of https://github.com/SeaQL/sea-orm/issues/521 but we don't need it now as we figure out another way to achieve it.

billy1624 commented 2 years ago

I'll close this now. In case anyone interested on this. Please leave a comment and consider reopen this :)