Something wrong with primary key index on table - Not expected

[mwhebert]

sample test below fails with "Something wrong with primary key index on table" which I would not expect.

let alasql = require('alasql');

let DemoDB = {
    appliance: [
        {name: 'A', type: 'Fast'},
        {name: 'B', type: 'Medium'},
        {name: 'C', type: 'Slow'}
    ],
    alerts: [
        {sev: 'Critical', appliance: 'A'},
        {sev: 'Major', appliance: 'A'},
        {sev: 'Minor', appliance: 'B'},
        {sev: 'Info', appliance: 'C'},
        {sev: 'Critical', appliance: 'C'},
        {sev: 'Major', appliance: 'B'},
        {sev: 'Minor', appliance: 'A'}
    ]
}

alasql('CREATE DATABASE [Demo]');
alasql('USE DATABASE [Demo]');

alasql("CREATE TABLE appliance (name STRING PRIMARY KEY, type STRING)");
//alasql("CREATE TABLE appliance (name STRING, type STRING)");

alasql("CREATE TABLE alerts (sev STRING, appliance STRING REFERENCES appliance(name))");
//alasql("CREATE TABLE alerts (sev STRING, appliance STRING)");

alasql('SELECT * INTO appliance FROM ?',[DemoDB.appliance]);
alasql('SELECT * INTO alerts FROM ?',[DemoDB.alerts]);

// alasql('SELECT * INTO appliance FROM JSON("appliance.json")');
//alasql.tables.appliance.data = DemoDB.appliance;
//alasql.tables.alerts.data = DemoDB.alerts;

console.log(alasql("SELECT * FROM appliance ORDER BY name DESC"));
console.log( alasql("SELECT * FROM alerts ORDER BY appliance"));

console.log("Delete alerts:",alasql("DELETE FROM alerts WHERE appliance = 'A'"));
//
// **** Fails here: Error: Something wrong with primary key index on table
//
console.log("Delete appliance:",alasql("DELETE FROM appliance WHERE name = 'A'"));

console.log(alasql("SELECT * FROM appliance ORDER BY name DESC"));
console.log( alasql("SELECT * FROM alerts ORDER BY appliance"));

[akumadare]

Hi, I had a quick look at this and the bug replication code can be distilled to just one table using a primary key:

let alasql = require('alasql');

let DemoDB = {
    appliance: [
        {name: "A", type: 'Fast'},
        {name: "B", type: 'Medium'},
        {name: "C", type: 'Slow'}
    ]
}

alasql('CREATE DATABASE [Demo]');
alasql('USE DATABASE [Demo]');

alasql("CREATE TABLE appliance (name STRING PRIMARY KEY, type STRING)");
alasql('SELECT * INTO appliance FROM ?',[DemoDB.appliance]);

console.log("Delete appliance:",alasql("DELETE FROM appliance WHERE name = 'A'"));

The error comes from within the table["delete"] function:

        if (this.pk) {
            var pk = this.pk;
            var addr = pk.onrightfn(r);
            if (typeof this.uniqs[pk.hh][addr] === 'undefined') {
                throw new Error('Something wrong with primary key index on table');
            } else {
                this.uniqs[pk.hh][addr] = undefined;
            }
        }

• pk.onrightfn(r); returns undefined
• uniqs[pk.hh]; is an empty object

[capelski]

Any updates on this issue? I'm still getting the same error when trying to delete from a table with a primary key

[RB-News-MailAccount]

Some comments:

• The issue does not only arise with deletion from tables with primary key after filling with 'SELECT INTO'-statements but also with deletion from tables having UNIQUE constraints after analog use of 'SELECT INTO’ before.
• There are two problems:

1. The root cause of the issue is 'SELECT INTO' logic circumventing the usual insert logic by doing a push directly on the table data (file 40select.js, lines 308-315 - at time of writing, code may change).

2. The second problem comes from the deletion: primary key and unique key indices are checked on deletion, and if one or the other does not exist an exception is thrown (file 60createtable.js, lines 544 and 553). The check in itself makes sense, but it is a check not on the users data (which may or may not be correct regarding the primary key or the uniqueness) but on the internals of the database. Hence, a user of the library would expect an error statement but no exception, making the ‘SELECT INTO’-logic unusable to him. Hence, I would prefer lines 544 and 553 of 60createtable.js to hold logging statements with additional information on the context (affected table etc.).

• The root issue with ‘SELECT INTO’ is similar to issue #92 with ‘INSERT INTO’ solved in April 2015 (where a direct concat of the data was used instead of insert).

• I made a quick fix replacing lines 308-315 of 40select.js with the following code:

query.intofns =
          "var db = alasql.databases['" +(this.into.databaseid || databaseid) + "'];" +

          "var table = db.tables['" + this.into.tableid + "'];" +

          "var cR = alasql.utils.cloneDeep(r);" +

          "if (table.defaultfns) {" +

          "  var defaultfns = 'return alasql.utils.extend(r,{' + table.defaultfns + '})';" +

          "  var defaultfn = new Function('r,db,params,alasql', defaultfns);" +

          "  defaultfn(cR, db, [], alasql);" +

          "}" +

          "if (table.insert) {" +

          "  table.insert(cR, false);" +

          "} else {" +

            "table.data.push(cR);" +

          "}";

It ran smooth with alasql tests, the test code given in this issue and in my own table with unique constraints. I have not tested tables with column defaults, yet. However, I am not comfortable with fix and do not directly create a pull request for it: I would prefer a more elaborated version - especially in the long run. On one hand, creating code with string and new Function (or eval()) is not the best solution as long as there are alternatives in a given context. On the other hand, code in ‘INSERT INTO’ and ‘SELECT INTO’ functionality have overlapping logic and an unified version without - partial - code duplication would be preferable.

Hope, it helps! Thank you for providing AlaSql!