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

Adjusting the transformers for schema support #23

Merged
merged 3 commits into from
Sep 26, 2024

Conversation

LittleDan9
Copy link
Contributor

When using a database with a schema defined in the SQLAlchemy metadata the resulting value of foreign_key.target_fullname would be [schema].[table_name].[column_name] The previous implementation split foreign_key.target_fullname on the . and defined left_table as element [0] ([schema]) and left_column as element [1] ([table_name]). This splitting does not take into account the possibility of a schema.

The table_name values defined in self.metadata.tables are stored as [schema].[table_name]. Therefore, the logic to enforce the existence of the table on the left side was always False when a schema is present because the names was not the same.

Changing the assumption to be that the last element [-1] of the resulting array when splitting foreign_key.target_fullname on a . is the column name, ensures that regardless of the use of a schema that left-column will be the correct value for column_name. Additionally, since the value of the table name stored in self.metadata.tables is foreign_key.target_fullname less .[column_name], by rejoining the elements of the split less the last element [:-1] by a . results in the correct value for left_table regardless of schema usage.

@tedivm
Copy link
Owner

tedivm commented Sep 26, 2024

If you run make chores it should resolve the formatting issues.

Tests currently aren't passing, but it looks like a fairly trivial issue there. If you can get that resolved so the formatting and testing passes I should be able to merge this.

Thank you!

@LittleDan9
Copy link
Contributor Author

Thanks @tedivm for the direction. I've updated the tests and ran make chores to correct formatting issues.. Let me know if there is anything else.

@tedivm tedivm merged commit 455e0fc into tedivm:main Sep 26, 2024
9 checks passed
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.

2 participants