ko4life-net / ko

Open source development of the game Knight Online. This is a reversed engineered old version of the game aiming to replicate the nostalgic experience we all once had <3
MIT License
54 stars 21 forks source link

Importing db throws an error with Turkish collation #223

Closed stevewgr closed 4 months ago

stevewgr commented 4 months ago

Description

We had one user experienced issues while trying to use the import.ps1 script in the ko-db project. The following errors were produced:

C:\Users\*\Desktop\ko-master\ko-master\src\db\src\procedure\CHECK_KNIGHTS.sql
Invoke-Sqlcmd : Must declare the scalar variable "@knightsindex".
Must declare the scalar variable "@knightsindex".
 Msg 137, Level 15, State 2, Procedure CHECK_KNIGHTS, Line 22.
At C:\Users\*\Desktop\ko-master\ko-master\src\db\import.ps1:60 char:10
+   return Invoke-Sqlcmd -ConnectionString $connection_string -InputFil ...
+          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (:) [Invoke-Sqlcmd], SqlPowerShellSqlExecutionException
    + FullyQualifiedErrorId : SqlError,Microsoft.SqlServer.Management.PowerShell.GetScriptCommand

Note that multiple other stored procedures also produced the same error.

The user is using Turkish as collation for its global default SQL Server installation.

Screenshots

image

To Reproduce

I was able to fix it by making sure @knightsindex variable is referenced in the same case sensitivity as it is declared, @KnightsIndex. However reproducing the issue to really understand the real cause of this requires reinstalling SQL Server with Turkish collation, which @twostars was kind enough to start a new VM and test this. He found out that it has to do with the Turkish dotted i character: https://en.wikipedia.org/wiki/Dotless_I because i and I are different characters in Turkish. However the reason it was hard to reproduce, because even if I set the new database with the same Turkish collation, I couldn't reproduce, because it relies on the actual SQL Server default installation collation, hence required reinstalling the SQL Server to actually reproduce. Odd behavior from Microsoft end.

Tasks

twostars commented 4 months ago

As far as the affected procs go, the 3 were (offhandedly): CHECK_KNIGHTS RANK_KNIGHTS EDITER_KNIGHTS

These specifically mix @knightsindex and @KnightsIndex which causes this issue to be exhibited with the 2nd i.

They're more than likely not the only cases with mixed case variable names, but they are the only ones able to cause this issue with this collation.

I assume for the sake of ensuring future similar errors don't occur with other collations you're intending to ensure all of their case matches. I can swap it to a case sensitive collation to definitively catch these if you haven't already.

stevewgr commented 4 months ago

@twostars good point. I'll do that while refactoring the procedures a bit. Doing some renames as well, to match with their coressponding data-types (e.g. smallint types starts with s) for consistency sake. Will provide that in a separate PR.

twostars commented 4 months ago

CHECK_KNIGHTS - @knightsindex

C:\Users\twostars\Desktop\ko\src\db\src\procedure\CHECK_KNIGHTS.sql
Invoke-Sqlcmd : Must declare the scalar variable "@knightsindex".
Must declare the scalar variable "@knightsindex".
 Msg 137, Level 15, State 2, Procedure CHECK_KNIGHTS, Line 22.

DELETE_CHAR - strCHARID1

 C:\Users\twostars\Desktop\ko\src\db\src\procedure\DELETE_CHAR.sql
Invoke-Sqlcmd : Invalid column name 'strCHARID1'.
 Msg 207, Level 16, State 1, Procedure DELETE_CHAR, Line 66.

EDITER_KNIGHTS - @Row, @knightsindex

C:\Users\twostars\Desktop\ko\src\db\src\procedure\EDITER_KNIGHTS.sql
Invoke-Sqlcmd : Must declare the scalar variable "@Row".
Must declare the scalar variable "@knightsindex".
 Msg 137, Level 15, State 2, Procedure EDITER_KNIGHTS, Line 25.

EXEC_KNIGHTS_USER - strUserID

C:\Users\twostars\Desktop\ko\src\db\src\procedure\EXEC_KNIGHTS_USER.sql
Invoke-Sqlcmd : Invalid column name 'strUserID'.
 Msg 207, Level 16, State 1, Procedure EXEC_KNIGHTS_USER, Line 13.

LOAD_CHAR_INFO - strUserID

C:\Users\twostars\Desktop\ko\src\db\src\procedure\LOAD_CHAR_INFO.sql
Invoke-Sqlcmd : Invalid column name 'strUserID'.
 Msg 207, Level 16, State 1, Procedure LOAD_CHAR_INFO, Line 21.

RANK_KNIGHTS - @Knightsindex, @knightsindex

C:\Users\twostars\Desktop\ko\src\db\src\procedure\RANK_KNIGHTS.sql
Invoke-Sqlcmd : Must declare the scalar variable "@Knightsindex".
Must declare the scalar variable "@knightsindex".
 Msg 137, Level 15, State 2, Procedure RANK_KNIGHTS, Line 20.