Fix for foreign key's referencing table schemas. #6169

tisdadd · 2025-01-14T01:29:57Z

Thanks for such a great project!

I used patch-package with the following when I ran into an issue with foreign keys utilizing schemas as mentioned in the title, so the basic fix for those who use that in the meantime is below.

diff --git a/node_modules/knex/lib/schema/tablebuilder.js b/node_modules/knex/lib/schema/tablebuilder.js
index 99f849a..5d78396 100644
--- a/node_modules/knex/lib/schema/tablebuilder.js
+++ b/node_modules/knex/lib/schema/tablebuilder.js
@@ -109,8 +109,9 @@ class TableBuilder {
             },
           };
         }
-        foreignData.inTable = pieces[0];
-        foreignData.references = pieces[1];
+        
+        foreignData.references = pieces.pop();
+        foreignData.inTable = pieces.join('.');
         return returnObj;
       },
       withKeyName(keyName) {

After reading the contributing guidelines, I also created some migrations and tests for them based off of my main project where I was actually running through my own checklist of tests. As I only have set up for mssql so far, I have not tested against other drivers.

As for why I did if(db === DRIVER_NAMES.MsSQL) { instead of if (isMsSql(knex)) {, through some digging I found that will always return false at that level, since knex is undefined there.

During the digging, it looked like test/util/db-helpers.js function getDriverName(knex) always had undefined for knex passed in, so I am not sure if the switch statements are working for other bits or not. If you would like me to dig into that, I can try to find time as well.

Anyway, thanks again and let me know if I should change anything.

tisdadd · 2025-01-14T01:43:24Z

Forgot to mention, that while searching for solutions to this I did find PR 5651as well, which handles something slightly different, and may not allow referencing another schema if needed. If both features are wanted, we may need to work through that. Microsoft has the WideWorldImporters example, which has several examples of Foreign Keys from the [Application] schema being utilized elsewhere, for instance [Sales].[Customers].[AlternateContactPersonID] references [Application].[People].[PersonID], so the ability to specify what schema is referenced should generally be good, but we may consider needing to just check if there is a schema here, and if not and there is a schema from withSchema then utilize that one as per the other PR.

Fix for foreign key's referencing table schemas.

2fac2b5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix for foreign key's referencing table schemas. #6169

Fix for foreign key's referencing table schemas. #6169

Uh oh!

Uh oh!

Uh oh!

Fix for foreign key's referencing table schemas. #6169

Are you sure you want to change the base?

Fix for foreign key's referencing table schemas. #6169

Conversation

Uh oh!

Uh oh!

Uh oh!