fix(mssql): use one-part name in sp_rename when creating a table with override=true#11976
fix(mssql): use one-part name in sp_rename when creating a table with override=true#11976Cogito wants to merge 1 commit intoibis-project:mainfrom
Conversation
… override=true
The `sp_rename` stored proc requires that the new name is just the table name. Previously we were supplying the fully qualified name and this was causing incorrect tables to be created.
For example, `conn.create_table("my_table", database=("my_db", "dbo"), override=true)' would create a table called `my_db.dbo.[my_db.dbo.my_table]`
Ref: https://learn.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-rename-transact-sql
|
Thank @Cogito, this very well could be a bug. We are going to need a test for this this though. Really, the test should already exist. It just doesn't yet. I created #11979 to track this. Once we have a PR landed that adds this test, then we will be ready to review and merge this PR. Would you be willing to take a look at that PR and try implementing it? I can also throw claude code at it, but I don't have much time to babysit it and push it over the line, so you would need to take charge here mostly. |
|
Hey @NickCrews yes I'll take a look at this. I'll note that the issue here is not that the database is specified as a tuple, the error happens if it is specified as a string as well. The issue comes when specifying override=true which means we need to test that too. Should that be the same issue or a new one? |
2 participants
Description of changes
Instead of using sqlglot's
table.sql()function to get the target table name, usetable.name.The
sp_renamestored proc requires that the new name is just the table name. Previously we were supplying the fully qualified name and this was causing incorrect tables to be created.For example,
conn.create_table("my_table", database=("my_db", "dbo"), override=true)' would create a table calledmy_db.dbo.[my_db.dbo.my_table]`Ref: https://learn.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-rename-transact-sql
Issues closed
I haven't raised an issue for this, and couldn't find one when searching.