nolanlawson / cordova-plugin-sqlite-2

Native SQLite database API for Cordova/PhoneGap/Ionic, modeled after WebSQL (UNMAINTAINED)
https://nolanlawson.com/2016/04/10/introducing-the-cordova-sqlite-plugin-2/
Apache License 2.0
168 stars 28 forks source link

null/not null of column in first row read from select overrides types of subsequent rows #33

Closed awwx closed 8 years ago

awwx commented 8 years ago

Tested on iOS 9.3 with cordova-plugin-sqlite-2 1.0.3

Consider a table with a column that can be null:

create table foo (
  id text not null primary key,
  x integer null
);

If a value in the first row read in a select statement is null, the column in subsequent rows is also read as null:

insert into foo values ('a', null), ('b', 1), ('c', 2);
select * from foo order by id;

{"id":"a","x":null}
{"id":"b","x":null}
{"id":"c","x":null}

Conversely, if the value in the first column is not null, subsequent rows are also read as not null:

delete from foo;
insert into foo values ('a', 1), ('b', null), ('c', null);
select * from foo order by id;

{"id":"a","x":1}
{"id":"b","x":0}
{"id":"c","x":0}

I suspect this may be because the column types are cached from the first row: https://github.com/nolanlawson/cordova-plugin-sqlite-2/blob/v1.0.3/src/ios/SQLitePlugin.m#L189

nolanlawson commented 8 years ago

Yup, this is a bug. If you could add a failing test (or even fix it), that would be very helpful. Else I will try to get to this when I can.


Nolan Lawson http://nolanlawson.com https://github.com/nolanlawson On Jun 18, 2016 6:35 AM, "Andrew Wilcox" notifications@github.com wrote:

Tested on 9.3 iOS with cordova-plugin-sqlite-2 1.0.3

Consider a table with a column that can be null:

create table foo ( id text not null primary key, x integer null );

If a value in the first row read in a select statement is null, the column in subsequent rows are also read as null:

insert into foo values ('a', null), ('b', 1), ('c', 2); select * from foo order by id;

{"id":"a","x":null} {"id":"b","x":null} {"id":"c","x":null}

Conversely, if the value in the first column is not null, subsequent rows are also read as not null:

delete from foo; insert into foo values ('a', 1), ('b', null), ('c', null); select * from foo order by id;

{"id":"a","x":1} {"id":"b","x":0} {"id":"c","x":0}

I suspect this may be because the column types are cached from the first row: https://github.com/nolanlawson/cordova-plugin-sqlite-2/blob/v1.0.3/src/ios/SQLitePlugin.m#L189

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nolanlawson/cordova-plugin-sqlite-2/issues/33, or mute the thread https://github.com/notifications/unsubscribe/AARUwqxtap5FS0TVk4qdSuhBh2m5YAUEks5qM_QWgaJpZM4I48s7 .

nolanlawson commented 8 years ago

OK, I can confirm the bug. It returns 0 instead of null. I have found a test to reproduce, but haven't fixed it yet.

nolanlawson commented 8 years ago

Hm, so to be honest, I'm not sure if we actually want to fix this issue. It looks like the only way to solve it on Android at least is to fetch the string version of every column and check whether or not it's null. This seems like a lot of overhead just to handle what is probably a very minor use case (how often are folks trying to distinguish null from 0 in an integer column?). It's a performance hit that I'm not sure we really need to take.

I guess for the special case of ints/floats and 0 values we could do an extra check to test whether it's null or not, though. That would limit the performance impact while still giving the correct behavior.

nolanlawson commented 8 years ago

OK yeah this is worse than I thought: if the first row is null, then all subsequent rows are interpreted as null. Definitely needs to be fixed.

awwx commented 8 years ago

(how often are folks trying to distinguish null from 0 in an integer column?)

Well, of course this is moot since you've already fixed it :), however, to answer your question... it's essential if null values are to be useful. (If I can't distinguish between null values and integers, I might as well just make the column "NOT NULL", and add a second column to use as a flag for whatever I meant null to mean).

In my case, I had a task ID, which was an integer (which could be zero), and NULL indicated that a task hadn't been created yet. Thus "a task hasn't been created yet" and "a task has been created with a task ID which happens to be 0" is quite different :)

nolanlawson commented 8 years ago

fixed in #35, will release soon. agree this is important