sea-query icon indicating copy to clipboard operation
sea-query copied to clipboard

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

Open nahuakang opened this issue 3 years ago • 2 comments

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:

  • TableCreateStatement
  • TableAlterStatement
  • TableDropStatement
  • TableRenameStatement
  • TableTruncateStatement
  • IndexCreateStatement
  • IndexDropStatement
  • ForeignKeyCreateStatement
  • ForeignKeyDropStatement
  • TypeCreateStatement
  • TypeAlterStatement
  • TypeDropStatement

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 avatar Sep 18 '22 10:09 nahuakang

@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 :)

nahuakang avatar Sep 18 '22 15:09 nahuakang

@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

billy1624 avatar Sep 19 '22 10:09 billy1624

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

ikrivosheev avatar Sep 30 '22 17:09 ikrivosheev

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

nahuakang avatar Sep 30 '22 18:09 nahuakang

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 avatar Oct 07 '22 07:10 billy1624

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

billy1624 avatar Oct 07 '22 07:10 billy1624