editfmah / sharkorm

Shark ORM for iOS/macOS/tvOS/watchOS
http://sharkorm.com
Other
248 stars 39 forks source link

Error while doing a rawQuery #101

Closed stefanosisto closed 6 years ago

stefanosisto commented 6 years ago

Hello. I have this error after the last pod update.

Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** +[NSString stringWithUTF8String:]: NULL cString'

How can I debug it to give you more info ?

stefanosisto commented 6 years ago

NSObject* value = [[SRKUtilities new] sqlite3_column_objc:statement column:p]; SharkORM.m , row 603.

Ok, using the exception breakpoint I found this as Null. But I don't know why. Before the last update, rawResults were Optional. Now not and now I have this crash. Also some of my mapper (in which I mapped the id value ) now are in errore because now id is an Optional (not before).

How can I restore the previous version of this pod until this big is fixed? And what's the exactly version of the previous one?

editfmah commented 6 years ago

Hi @stefanosisto,

That particular piece of code has not changed, but it may be related to the column type code. It should be impossible for that particular piece of code to crash as the value is checked, the DB tells us it it s String, or there is a column data type which is incorrect and it falls through. If it ends up with no value then an NSNull is returned which would still be valid so I am at a complete loss as to how you could see this error.

What would be super useful is if you could just step through - (id)sqlite3_column_objc:(sqlite3_stmt *)stmt column:(int)i, I would really need to know what value sqlite3_column_type(stmt, i) returns, as that is key to solving this.

That said, if the column is indeed null then I would expect to see this result. Do you happen to have a line number for your initial crash NSInvalidArgumentException', reason: '*** +[NSString stringWithUTF8String:]: NULL cString, as it may be that some code is expecting a string but in fact has received a NULL instead. Or the string is zero bytes in length which would be very weird, but I guess possible.

If you wish to revert to the previous version then, that would be v2.1.3.

As for optional Id, i'm not so sure about reverting this as that property is indeed nullable and should really remain so.

editfmah commented 6 years ago

Also the raw results are not optional because you always get an object back, even with no results. This should have no bearing on your particular issues.

stefanosisto commented 6 years ago

Ok, i reverted to 2.1.0

The fact are:

" What would be super useful is if you could just step through - (id)sqlite3_column_objc:(sqlite3_stmt *)stmt column:(int)i, I would really need to know what value sqlite3_column_type(stmt, i) returns, as that is key to solving this. "

What do you need to debug this?

"That said, if the column is indeed null then I would expect to see this result. Do you happen to have a line number for your initial crash NSInvalidArgumentException', reason: '*** +[NSString stringWithUTF8String:]: NULL cString, as it may be that some code is expecting a string but in fact has received a NULL instead. Or the string is zero bytes in length which would be very weird, but I guess possible."

Yes, columname is null while it's in the loop.

editfmah commented 6 years ago

id column was always nullable because it's an objective-c property and an object. Swift, incorrectly decided to default that to id! because we had not specified any nullability for it. We absolutely cannot make it !, because it is nullable and indeed is required to be so by any code using GUID's as a primary key type so i'm not really sure we could revert this easily.

Bugs aside, the idea of this release was to strongly type the interface as best as we could. Sadly Swift made assumptions which were actually incorrect and could easily cause crashes if misused.

I will investigate the raw query issue, as it looks like the Schema Manager might be returning the wrong data types for objects which are not managed.

I will add a test case and post a fix up soon.

stefanosisto commented 6 years ago

Okok. Write me your email and i give you my 3-4 .swift class. So you can test on it if you want.

editfmah commented 6 years ago

Hi, it's devs@sharksync.io.

editfmah commented 6 years ago

I have added a simple unit test for raw queries, i'm not seeing any issues so far. So i'll await your email and see if there is something more specific we would be investigating.

- (void)test_raw_query {

    [SharkORM rawQuery:@"DROP TABLE IF EXISTS RawQueryTest;"];
    [SharkORM rawQuery:@"CREATE TABLE RawQueryTest (Id INTEGER PRIMARY KEY AUTOINCREMENT, value TEXT, numValue INTEGER, blobValue BLOB);"];

    XCTAssert([SharkORM rawQuery:@"SELECT * FROM RawQueryTest"] != nil, @"Raw Query object was nil");

    XCTAssert([SharkORM rawQuery:@"SELECT * FROM RawQueryTest"].rowCount == 0, @"RawQuery, row count was incorrect");

    [SharkORM rawQuery:@"INSERT INTO RawQueryTest (value, numValue) VALUES ('testing123',123);"];
    XCTAssert([SharkORM rawQuery:@"SELECT * FROM RawQueryTest"].rowCount == 1, @"RawQuery, row count was incorrect");

    XCTAssert([[[SharkORM rawQuery:@"SELECT * FROM RawQueryTest"] valueForColumn:@"value" atRow:0] isEqualToString:@"testing123"], @"RawQuery, retrieved value is incorrect");
}
alldritt commented 6 years ago

I'm experiencing this problem with this code which worked previously:

let dataVersion = SharkORM.rawQuery("PRAGMA data_version").value(forColumn: "data_version", atRow: 0) as? NSNumber

Debugging through the code, the problem seems to be in sqlite3_column_objc which assumes that every column will be associated with a table:

            NSString* tableName = [NSString stringWithUTF8String:sqlite3_column_table_name(stmt, i)];
            if (tableName) {

The crash is happening because sqlite3_column_table_name is returning NULL which causes +[NSString stringWithUTF8String:] to throw an exception. This pattern is repeated several times in sqlite3_column_objc.

This against SharkORM (2.3.60)

editfmah commented 6 years ago

Thanks @alldritt, I have made a change for that path. It will be pushed as soon as I can get a rather sticky unit test to pass. Thanks Guys.

editfmah commented 6 years ago

@alldritt @stefanosisto , okay that has been built and pushed to Cocoapods. Let me know if there are any other issues or suggested changes.

Many thanks, Adrian.

alldritt commented 6 years ago

The problem I was experiencing has been resolved - thank you!

stefanosisto commented 6 years ago

Solved !!! Thanks !!!