Alasql: DELETE not working without WHERE xxx

Created on 5 Apr 2017 · 22Comments · Source: agershun/alasql

delete from statement dont work if i dont add where.
so I had to do work around like 'delete from SomeKindOfTable where 1=1'

! Feature request Good first issue Help wanted SQL compliance

Source

peon501

All 22 comments

Thanks for letting us know. Its really good that you check things like this so the library can get better!

mathiasrw on 5 Apr 2017

👍1

@mathiasrw is mentoring available to help tackle this? I know SQL and JS pretty well but I'm not as familiar with your codebase.

morrme on 8 Apr 2017

For sure!

The code base is the beast here... not SQL or JS.

Lets work on it together.

As a start I suggest that you follow the first steps in https://github.com/agershun/alasql/blob/develop/CONTRIBUTING.md until you have a new test that fails and then make a PR so we can have a good place to talk about things.

Future input: I have a feeling we have to find out if https://github.com/agershun/alasql/blob/7c528a44b9f01b170f764a95d338576bf894ae99/src/72delete.js#L25 is true even if no WHERE was set...

mathiasrw on 8 Apr 2017

👍1

@mathiasrw Thanks! I ran npm test before I started with anything, just for a baseline and got the failure below. Is this expected?

1411 passing (33s)
  376 pending
  1 failing

  1) Test 140 JavaScript Functions 7. Create table with default conversion Date:

      AssertionError: [ 5, 6 ] deepEqual [ 6, 6 ]
      + expected - actual

       [
      -  5
         6
      +  6
       ]

      at Context.<anonymous> (test140.js:105:10)



npm ERR! Test failed.  See above for more details.

morrme on 8 Apr 2017

Hmmmmm

Strange. The test should always pass before pushing to develop. (last push did not fail on travis)

What version of node are you using? WIndows? mac? Linux? Lets get this sorted...

mathiasrw on 9 Apr 2017

@mathiasrw
node: v6.10.0
npm: 3.10.10
OS: Windows -- I've tried it on two different computers, Windows 8 and Windows 10...same test fails.

Can you (or anyone) try building it and running npm test ?

morrme on 10 Apr 2017

It works on my computer right now.

I have a feeling its related to timezone. (dates are given in UTC but methods on it works on local time)

Can you try to change 7. Create table with default conversion Date test function to the following and let me know if this works out for you

        delete alasql.fn.Date;
        alasql('CREATE TABLE four (d Date)');

        alasql('INSERT INTO four VALUES ("2014-06-02"), ("2015-06-03")');

        var res = alasql("SELECT COLUMN YEAR(d) FROM four");
        assert.deepEqual(res,[2014,2015]);

        var res = alasql("SELECT COLUMN MONTH(d) FROM four");
        assert.deepEqual(res,[6,6]);

        var res = alasql("SELECT COLUMN DAY(d) FROM four");
        assert.deepEqual(res,[2,3]);

//      console.log(res);
        var res = alasql("SELECT COLUMN d FROM four");
        assert.deepEqual(res,["2014-06-02","2015-06-03"]);

(date changes from first and second day to second and 3rd day of june)

mathiasrw on 10 Apr 2017

results:

Test 140 JavaScript Functions 7. Create table with default conversion Date:

      AssertionError: [ 1, 2 ] deepEqual [ 2, 3 ]
      + expected - actual

       [
      -  1
         2
      +  3
       ]

      at Context.<anonymous> (test140.js:128:10)

morrme on 10 Apr 2017

Hmm. I see. It _is_ the timezone.

Try using this updated version of the test

https://github.com/agershun/alasql/blob/develop/test/test140.js#L107-L108

mathiasrw on 11 Apr 2017

@mathiasrw Success!

  1412 passing (12s)
  376 pending

morrme on 11 Apr 2017

@mathiasrw So the next step: Create a test to fail. I created a new test file and tested the following:

DELETE to work with a valid WHERE clause
DELETE to fail with an invalid WHERE clause
DELETE to work without WHERE clause

and they all passed.

I also tried this using the console (http://alasql.org/console ) and it works without WHERE there as well.

morrme on 11 Apr 2017

did you fixed bug ? on not ?

peon501 on 11 Apr 2017

if u cant reproduce here I done example:

<!DOCTYPE html>
<html lang="lt">
<head>
    <meta charset="utf-8">
    <title>TEST</title>
    <script type="text/javascript" src="/alasql.min.js"></script>
</head>
<body>
<script>
alasql('CREATE INDEXEDDB DATABASE IF NOT EXISTS testdb; ATTACH INDEXEDDB DATABASE testdb;USE testdb;', [], function(){
    alasql("CREATE TABLE IF NOT EXISTS Something (id,text);",[],function(){
        alasql('delete  from `Something` ',[],function(){
            alasql('insert into `Something`  ( id,text) values (1,"test") ',[],function(){
                console.log('ok');
            });
        });
    });
});
</script>
</body>
</html>

you will get error but if u will add

alasql('delete  from `Something`  where 1=1',[],function(){

everything works.

peon501 on 11 Apr 2017

Could this is be a INDEXEDDB related problem?

@peon501 As a side note I recommend using the lazy promise notation for async calls:

alasql([    'CREATE INDEXEDDB DATABASE IF NOT EXISTS testdb; ATTACH INDEXEDDB DATABASE testdb;USE testdb;',
        "CREATE TABLE IF NOT EXISTS Something (id,text);",
        'delete  from `Something` ',
        'insert into `Something`  ( id,text) values (1,"test")'
    ]).then(function(){
        console.log('ok');
    });

mathiasrw on 11 Apr 2017

yea I know, your way are good.
I just found there bug by doing other way with callbacks in callbacks so I posted it here.

peon501 on 11 Apr 2017

This problem coud occur for someone if they have to do some stuff in between deleting.

so I think someone should check why this happens, maybe dirty way but sometimes usefull.

peon501 on 11 Apr 2017

@mathiasrw sidebar: while researching this i found some links in the wiki that could/should be cleaned up. is someone already working on this?

morrme on 11 Apr 2017

This problem coud occur for someone if they have to do some stuff in between deleting.

@peon501 Can you explain this in a code example?

Please give me a hint to any wiki things that should be fixed.

Im working on getting the documentation into https://github.com/AlaSQL/alasql-docs so a simple PR could do it, but until its done just post it here or in a new issue.

mathiasrw on 12 Apr 2017

@mathiasrw if you want the wiki to stay current while you work on the docs project, i can send a PR--its mostly correcting or adding links.

as for this issue, i am happy to continue helping if we discover the issue that needs fixing.

morrme on 12 Apr 2017

Can you send PR to a wiki on github? Sure - bring it ON!

mathiasrw on 14 Apr 2017

👍1

Sorry, Still not working for me
Uncaught TypeError: Cannot read property 'tables' of undefined
at Function.LS.storeTable (alasql.js:16717)
at Function.LS.saveTableData (alasql.js:17032)
at statement (alasql.js:13326)
at Function.alasql.dexec (alasql.js:4575)
at Function.alasql.exec (alasql.js:4557)
at alasql (alasql.js:133)
at :1:1
LS.storeTable @ alasql.js:16717
LS.saveTableData @ alasql.js:17032
statement @ alasql.js:13326
alasql.dexec @ alasql.js:4575
alasql.exec @ alasql.js:4557
alasql @ alasql.js:133
(anonymous) @ VM68:1

hakeemajaj on 28 Apr 2017

😕2

https://github.com/agershun/alasql/blob/7c528a44b9f01b170f764a95d338576bf894ae99/src/72delete.js#L110
it seems a conditions is missing here: "&& db.engineid == 'LOCALSTORAGE')"
because a request like 'DELETE FROM table;' cause this issue: TypeError: alasql.engines[db.engineid].loadTableData is not a function