nwnxee / unified

Binaries available under the Releases tab on Github
https://nwnxee.github.io/unified
GNU General Public License v3.0
129 stars 92 forks source link

Update inc_array.nss to remove PRIMARY KEY status of index #1687

Closed Tildryn closed 1 year ago

Tildryn commented 1 year ago

PRIMARY KEY status of index prevents INSERT operations from working correctly, as when shifting the entries' positions the operation fails due to the constraint.

ELadner commented 1 year ago

I'm not sure this is the correct fix. Allowing duplicate index values can cause nasty side effects if, for example, duplicate keys are inserted mistakenly, when you can an Array_Erase with an index value, it will delete ALL entries from the array that have that index value.

Tildryn - can you give some more details about specific failures in shifting positions and the errors it produces?

On Tue, Jul 25, 2023 at 2:40 AM Daz @.***> wrote:

Merged #1687 https://github.com/nwnxee/unified/pull/1687 into master.

— Reply to this email directly, view it on GitHub https://github.com/nwnxee/unified/pull/1687#event-9908199024, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4C3KS2LILEIMNKJLH2NRLXR5Z5LANCNFSM6AAAAAA2WCQXF4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Eric Ladner

Tildryn commented 1 year ago

It shouldn't be an issue since the 'ind' field for the index is an internal implementation detail. The user never manipulates the 'ind' field in the table directly. Inserting multiple instances of the same key will result in two records with different indices.

The issue is here, in Array_Insert_Str:

if (index >= 0 && index <= rows) {
    // index is passed as an integer, so immune (as far as I know) to SQL injection for a one shot query.
    ExecuteStatement("UPDATE "+GetTableName(tag, obj)+" SET ind = ind + 1 WHERE ind >= "+IntToString(index), obj);
    // Element, however, is not.
    string stmt = "INSERT INTO "+GetTableName(tag, obj)+" VALUES ( @ind, @element )";
    sqlquery sqlQuery = SqlPrepareQueryObject(GetModule(), stmt);
    SqlBindInt(sqlQuery, "@ind", index);
    SqlBindString(sqlQuery, "@element", element);
    SqlStep(sqlQuery);
}

Since this attempts to update with 'ind = ind + 1', it attempts to change 'ind' to one that already exists (the index above it), and this fails due to the PRIMARY KEY constraint on 'ind'.