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

Added fk_name parameter to ormar.ForeignKey #849

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nikelborm
Copy link

@nikelborm nikelborm commented Sep 26, 2022

It will allow users to override automatically generated foreign key constraint name (for example in case when generated constraint name is too long for some DB)

In my case this code

...
class UserChosenConfiguration(ormar.Model):
  class Meta(BaseMeta):
    tablename = "user_chosen_configuration"
    constraints = [
      ormar.UniqueColumns("widget_instance_id", "widget_configuration_parameter_id", name="uc_user_chosen_configuration_pair")
    ]

  id: int = ormar.Integer(name="user_chosen_configuration_id", nullable=False, primary_key=True, autoincrement=True)

  widgetInstance: Optional[WidgetInstance]                             = ormar.ForeignKey(WidgetInstance,               name="widget_instance_id",                nullable=False, unique=False)
  widgetConfigurationParameter: Optional[WidgetConfigurationParameter] = ormar.ForeignKey(WidgetConfigurationParameter, name="widget_configuration_parameter_id", nullable=False, unique=False)
  value: str                                                           = ormar.Text(                                    name="value",                             nullable=False)
...

when used to generate migration causes such alembic migration:

...
def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    ...
    op.create_table('user_chosen_configuration',
    sa.Column('user_chosen_configuration_id', sa.Integer(), nullable=False),
    sa.Column('widget_instance_id', sa.Integer(), nullable=False),
    sa.Column('widget_configuration_parameter_id', sa.CHAR(32), nullable=False),
    sa.Column('value', sa.Text(), nullable=False),
    sa.ForeignKeyConstraint(['widget_configuration_parameter_id'], ['widget_configuration_parameter.widget_configuration_parameter_id'], name='fk_user_chosen_configuration_widget_configuration_parameter_widget_configuration_parameter_id_widgetConfigurationParameter'),
    sa.ForeignKeyConstraint(['widget_instance_id'], ['widget_instance.widget_instance_id'], name='fk_user_chosen_configuration_widget_instance_widget_instance_id_widgetInstance'),
    sa.PrimaryKeyConstraint('user_chosen_configuration_id'),
    sa.UniqueConstraint('widget_instance_id', 'widget_configuration_parameter_id', name='uc_user_chosen_configuration_pair')
    )
    # ### end Alembic commands ###
...

and when I try to execute it, migration fails with such error:

(base) nikel@nikel-A35S:~/tda-dashboards/database_experiment$ cd ~/tda-dashboards/database_experiment; DATABASE_HOST=$(docker inspect -f '{{range.NetworkSettings.Networks}}{{.IPAddress}}{{end}}' mysql-experiment) DATABASE_PORT="3306" DATABASE_USERNAME="root" DATABASE_PASSWORD="password" DATABASE_NAME="test" alembic upgrade head
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 1a681990b866, initial experiments
Traceback (most recent call last):
  File "/home/nikel/.local/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1819, in _execute_context
    self.dialect.do_execute(
  File "/home/nikel/.local/lib/python3.10/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
    cursor.execute(statement, parameters)
  File "/usr/lib/python3/dist-packages/MySQLdb/cursors.py", line 209, in execute
    res = self._query(query)
  File "/usr/lib/python3/dist-packages/MySQLdb/cursors.py", line 315, in _query
    db.query(q)
  File "/usr/lib/python3/dist-packages/MySQLdb/connections.py", line 239, in query
    _mysql.connection.query(self, query)
MySQLdb._exceptions.OperationalError: (1059, "Identifier name 'fk_user_chosen_configuration_widget_configuration_parameter_widget_configuration_parameter_id_widget' is too long")

so I found a way to fix it, by adding fk_name parameter to ormar.ForeignKey
and it allows me to specify foreign constraint name like this:

...
class UserChosenConfiguration(ormar.Model):
  class Meta(BaseMeta):
    tablename = "user_chosen_configuration"
    constraints = [
      ormar.UniqueColumns("widget_instance_id", "widget_configuration_parameter_id", name="uc_user_chosen_configuration_pair")
    ]

  id: int = ormar.Integer(name="user_chosen_configuration_id", nullable=False, primary_key=True, autoincrement=True)

  widgetInstance: Optional[WidgetInstance]                             = ormar.ForeignKey(WidgetInstance,               name="widget_instance_id",                nullable=False, unique=False, fk_name="asd2")
  widgetConfigurationParameter: Optional[WidgetConfigurationParameter] = ormar.ForeignKey(WidgetConfigurationParameter, name="widget_configuration_parameter_id", nullable=False, unique=False, fk_name="asd1")
  value: str                                                           = ormar.Text(                                    name="value",                             nullable=False)
...

and it creates migration which executes successfully:

...
op.create_table('user_chosen_configuration',
    sa.Column('user_chosen_configuration_id', sa.Integer(), nullable=False),
    sa.Column('widget_instance_id', sa.Integer(), nullable=False),
    sa.Column('widget_configuration_parameter_id', sa.CHAR(32), nullable=False),
    sa.Column('value', sa.Text(), nullable=False),
    sa.ForeignKeyConstraint(['widget_configuration_parameter_id'], ['widget_configuration_parameter.widget_configuration_parameter_id'], name='asd1'),
    sa.ForeignKeyConstraint(['widget_instance_id'], ['widget_instance.widget_instance_id'], name='asd2'),
    sa.PrimaryKeyConstraint('user_chosen_configuration_id'),
    sa.UniqueConstraint('widget_instance_id', 'widget_configuration_parameter_id', name='uc_user_chosen_configuration_pair')
    )
...

nikelborm added 2 commits September 26, 2022 23:53
It will allow users to override automatically generated foreign key constraint name (for example in case when generated constraint name is too long for some DB)
@nikelborm nikelborm marked this pull request as ready for review September 26, 2022 21:28
@nikelborm
Copy link
Author

If I need to extend documentation, please point me where I can edit it.

@nikelborm
Copy link
Author

Added fk_name to many-to-many because it has the same problem with too long names

...
class Ticker(ormar.Model):
  class Meta(BaseMeta):
    tablename = "ticker"

  id: int = ormar.Integer(name="ticker_id", nullable=False, primary_key=True, autoincrement=True)

  code: str                = ormar.String(name="code",                  nullable=False, unique=True, max_length=10)

class TickerToTickerCollection(ormar.Model):
  class Meta(BaseMeta):
    tablename = "ticker_to_ticker_collection"

class TickerCollection(ormar.Model):
  class Meta(BaseMeta):
    tablename = "ticker_collection"

  id: int = ormar.Integer(name="ticker_collection_id", nullable=False, primary_key=True, autoincrement=True)
  tickers: Optional[List[Ticker]] = ormar.ManyToMany(
    to=Ticker,
    through=TickerToTickerCollection,
    related_name="tickerCollections",
    # through_relation_fk_name="asd1",
    # through_reverse_relation_fk_name="asd2",
    through_relation_name="ticker_collection_id",
    through_reverse_relation_name="ticker_id",
  )
...

previous code causes migration to fail with the same "too long constraint name" error

...
op.create_table('ticker_to_ticker_collection',
    sa.Column('id', sa.Integer(), nullable=False),
    sa.Column('ticker_id', sa.Integer(), nullable=True),
    sa.Column('ticker_collection_id', sa.Integer(), nullable=True),
    sa.ForeignKeyConstraint(['ticker_collection_id'], ['ticker_collection.ticker_collection_id'], name='fk_ticker_to_ticker_collection_ticker_collection_ticker_collection_id_ticker_collection_id', onupdate='CASCADE', ondelete='CASCADE'),
    sa.ForeignKeyConstraint(['ticker_id'], ['ticker.ticker_id'], name='fk_ticker_to_ticker_collection_ticker_ticker_id_ticker_id', onupdate='CASCADE', ondelete='CASCADE'),
    sa.PrimaryKeyConstraint('id')
    )
...

but if I uncomment

...
    through_relation_fk_name="asd1",
    through_reverse_relation_fk_name="asd2",
...

asd1 and asd2 will override constraint names in generated ticker_to_ticker_collection table in migration

@collerek
Copy link
Owner

There are missing tests.
Also, I believe it will fail with inheritance as you would end up with two models with the same fk name.

@Stvchm9703
Copy link

do it end? current I got hit by this issue

@nikelborm
Copy link
Author

@Stvchm9703 I dont have time to finish it now. If You want you can add missing tests. I am not very proficient in python, and I wrote even this little part of code with some effort. And I wanted to ask a little help here

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.

3 participants