Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PostgreSQL] Obtaining Null values from varchar/text columns in DB results in Optional("") being returned #211

Closed
Cyfirr opened this issue Nov 2, 2016 · 5 comments
Assignees

Comments

@Cyfirr
Copy link

Cyfirr commented Nov 2, 2016

Optional("") returned where nil is expected

Based on TodoBackend project (https://github.com/Zewo/TodoBackend) I did very similar PSQL store for my project. When my model contains String? fields replicating the null-allowed text or varchar columns in DB, if I insert nil values when writing it to DB I expect the nil values to be returned by the get methods as well. DB correctly has NULL values in it, but values returned by get(id) or getAll() methods are Optional("") instead of nil.

Steps to reproduce

  1. Create a model having the String? fields and corresponding table in DB having text or varchar fields which can be NULLs.
  2. In your tests / main / whatever executable source file instantiate your model and assign nil values to fields of type String?.
  3. Insert the record using the model and store.insert(model) method.
  4. Save the value returned by insert into a variable.
  5. Compare model object created by you and model returned by insert within the PersistedEntity.

Expected: nil values are nil values in both and comparison succeeds.
Actual: nil values were replaced by Optional("")

My code:

func testAssetInsertion() throws {
    let original = Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil) 
    let inserted = try store.insertAsset(asset: original)
    XCTAssertEqual(inserted.model, original)
    let retrieved = try store.getAsset(id: "bc_id")
    XCTAssertEqual(retrieved!.model, original)
    let retrievedThroughGetAll = try store.getAllAssets()
    XCTAssertEqual(retrievedThroughGetAll.count, 1)
    XCTAssertEqual(retrievedThroughGetAll[0].model, original)
}

Test results:

/home/anatoli/workspace/StorageService/Tests/StorageLayerTests/PostgreSQLEBooksStore/PostgreSQLEBooksStoreTests.swift:45: error: PostgreSQLEBooksStoreTests.testAssetInsertion : XCTAssertEqual failed: ("Asset(id: "bc_id", name: "Test", frontCover: Optional(""), data: Optional(""), type: Optional(""))") is not equal to ("Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil)") - 
/home/anatoli/workspace/StorageService/Tests/StorageLayerTests/PostgreSQLEBooksStore/PostgreSQLEBooksStoreTests.swift:47: error: PostgreSQLEBooksStoreTests.testAssetInsertion : XCTAssertEqual failed: ("Asset(id: "bc_id", name: "Test", frontCover: Optional(""), data: Optional(""), type: Optional(""))") is not equal to ("Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil)") - 
/home/anatoli/workspace/StorageService/Tests/StorageLayerTests/PostgreSQLEBooksStore/PostgreSQLEBooksStoreTests.swift:50: error: PostgreSQLEBooksStoreTests.testAssetInsertion : XCTAssertEqual failed: ("Asset(id: "bc_id", name: "Test", frontCover: Optional(""), data: Optional(""), type: Optional(""))") is not equal to ("Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil)")
  • I have implementation of insert and model very close to what is in TodoStore and related sources.
    So I expect it to be reproducible there.
    If not - please let me know and I will add more info.
@Danappelxx Danappelxx self-assigned this Nov 2, 2016
@Danappelxx
Copy link
Member

Reproduced, looking into it now. Thanks for the thorough bug report!

@Danappelxx
Copy link
Member

Fixed in ZewoGraveyard/PostgreSQL@c009b18, tagged 0.14.1. Try swift build --clean=dist and running the project again!

@Cyfirr
Copy link
Author

Cyfirr commented Nov 3, 2016

Verified for the case:
let original = Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil)
However, if I will change the inserted entity to
let original = Asset(id: "bc_id", name: "Test", frontCover: "", data: "", type: "")
Then test will fail. DB should have an empty string in my opinion and empty string should be returned by the ORM.
/home/anatoli/workspace/StorageService/Tests/StorageLayerTests/PostgreSQLEBooksStore/PostgreSQLEBooksStoreTests.swift:45: error: PostgreSQLEBooksStoreTests.testAssetInsertion : XCTAssertEqual failed: ("Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil)") is not equal to ("Asset(id: "bc_id", name: "Test", frontCover: Optional(""), data: Optional(""), type: Optional(""))") -
/home/anatoli/workspace/StorageService/Tests/StorageLayerTests/PostgreSQLEBooksStore/PostgreSQLEBooksStoreTests.swift:47: error: PostgreSQLEBooksStoreTests.testAssetInsertion : XCTAssertEqual failed: ("Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil)") is not equal to ("Asset(id: "bc_id", name: "Test", frontCover: Optional(""), data: Optional(""), type: Optional(""))") -
/home/anatoli/workspace/StorageService/Tests/StorageLayerTests/PostgreSQLEBooksStore/PostgreSQLEBooksStoreTests.swift:50: error: PostgreSQLEBooksStoreTests.testAssetInsertion : XCTAssertEqual failed: ("Asset(id: "bc_id", name: "Test", frontCover: nil, data: nil, type: nil)") is not equal to ("Asset(id: "bc_id", name: "Test", frontCover: Optional(""), data: Optional(""), type: Optional(""))")

So, probably a bit better distinguish is required between NULL and ""...

@Danappelxx
Copy link
Member

Reminder for self - need to add a check using PQgetisnull

@Danappelxx
Copy link
Member

Okay, really fixed this time. ZewoGraveyard/PostgreSQL@a3dbab5 @0.14.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants