vapor / sqlite-kit Goto Github PK

Currently on 3.0.0 RC2.3, it appears that Bool is unusable as an SQLite storage type. Using a model like this below, results in an error:

FluentSQLiteError.queryDataSerialize: Cannot serialize Bool to SQLiteData (SQLiteDatabase+Query.swift:104)

struct Project: Content, SQLiteUUIDModel, Migration {
    
    //MARK: Properties
    var id: UUID?
    var name: String
    var isActive: Bool
    var groupID: UUID?
}

I can fix the issue by creating an extension on Bool to conform to SQLiteDataConvertible, similar to:

enum SQLiteBoolError: Error {
    case inconvertibleBool
}

extension Bool: SQLiteDataConvertible {
    
    /// See `SQLiteDataConvertible.convertFromSQLiteData(_:)`
    public static func convertFromSQLiteData(_ data: SQLiteData) throws -> Bool {
        switch data {
        case .integer(let integer): return integer != 0 ? true : false
        default: throw SQLiteBoolError.inconvertibleBool
        }
    }
    
    /// See `convertToSQLiteData()`
    public func convertToSQLiteData() throws -> SQLiteData {
        let state = self == false ? 0 : 1
        return SQLiteData.integer(state)
    }
}

Is the inability to use Bool by default intentional? If not, is this a worthwhile enhancement to submit as a PR?

I have no idea how easy this is to fix, but currently an exception is thrown when an integer that exceeds Int32.max is passed to bind(_:) (https://github.com/vapor/sqlite/blob/master/Sources/SQLite/SQLite%2BStatement.swift#L39-L43). This (seemingly) makes it impossible to store 64-bit integers, which SQLite does natively support.

I noticed two issues:

1. Need to use SQLRaw("json") for column names to access data.... e.g.

                let firstFuture = Storage.shared.db.select()
                    .column(SQLRaw("json"))
                    .from("SoftwareStatements")
                    .where(SQLColumn(SQLRaw("id")), .equal, SQLBind(softwareStatementId))
                    .all()

2. Cannot access JSON data stored as text rather than 'blob' since attempt like

let result: SQLRow = returnedFromFuture
let softwareStatement = try! result.decode(column: "json", as: SoftwareStatement.self)

triggers this fatal error in SQLiteDataDecoder

            guard case .blob(var buffer) = self.data else {
                fatalError()
            }

with no other way exposed to obtain the text read from the database....

Would be grateful if, in relation to (2), a method was exposed on SQLRow to access or decode SQLiteData of type text from database.

Describe the bug

if a struct has a nillable column, the encoder will skip the column, breaking an assert.

Basically, if

struct Plant: Codable {
    let name: String
    let color: String?
}

and

let plants: [Plant] = [
  Plant(name: "red bean", color: "red"),
  Plant(name: "green bean", color: "green"),
  Plant(name: "black bean", color: "black"),
  Plant(name: "mushroom", color: nil)
]

then when trying to insert the "mushroom", i'll get an error: Assertion failed: Column count (2) did not equal value count (1): Plant(name: "mushroom", color: nil).: file SQLKit/SQLInsertBuilder.swift, line 54

To Reproduce

Sample project illustrating this bug is here: https://github.com/danramteke/CodableSQLiteKit

Expected behavior

A clear and concise description of what you expected to happen.

@vzsg suggested filing a bug report: https://discord.com/channels/431917998102675485/684159753189982218/797463454139023380

Environment

• Vapor Framework version:
• Vapor Toolbox version:
• OS version: Big Sur

Additional context

Add any other context about the problem here.

Please do NOT use the name "SQLite" for your project.

"SQLite" is a registered trademark. "SQLite" refers to the underlying database engine upon which you have built a wrapper. By calling your project "SQLite", you are confusing people about what software they are running. Using the name "SQLite" also implies that you are associated with the original SQLite project, which is dishonest.

You are welcomed to use "SQLite" as part of the name for your software, so that people know that it uses SQLite. For example, you might call your software "SQLite Wrapper for Swift" or "Vapor's Swift SQLite Wrapper". But you may not use the unqualified name "SQLite".

As discussed in Discord:
Foreign keys are disabled in SQLite by default and need to be enabled.
PRAGMA foreign_keys = ON;
Check this old issue for more details: vapor/fluent-sqlite-driver#9

I would suggest to bring back the methods enableReference and disableReference from Vapor 3, but also set it enabled by default for SQLite instances.

I have two models, Bite and Tag. They have a many-to-many relationship to each other, so I've created a BiteTagPivot for easy filtering, which returns siblings().

I have created two Bites, which share 3 of the same Tags with a fourth Tag being unique to the second Bite. eg:

"Bite1": {
    "tags": ["my", "first", "bite"]
}

"Bite2" {
    "tags": ["my", "first", "bite", "man"]
}

I then have a route set up to return Future<[Tag]>:

func getBiteTagsHandler(_ req: Request) throws -> Future<[Tag]> {
        
    return try req.parameter(Bite.self).flatMap(to: [Tag].self) { bite in
            
        return try bite.tags.query(on: req).all()
    }
}

When running this query, I get the following responses:

bites/1/tags

[
    {
        "id": 1,
        "title": "my"
    },
    {
        "id": 2,
        "title": "first"
    },
    {
        "id": 3,
        "title": "bite"
    }
]

bites/2/tags

[
    {
        "id": 4,
        "title": "my"
    },
    {
        "id": 5,
        "title": "first"
    },
    {
        "id": 6,
        "title": "bite"
    },
    {
        "id": 7,
        "title": "man"
    }
]

Even though there are only 4 tags in my database:

/tags

[
    {
        "id": 1,
        "title": "my"
    },
    {
        "id": 2,
        "title": "first"
    },
    {
        "id": 3,
        "title": "bite"
    },
    {
        "id": 4,
        "title": "man"
    }
]

The second bite has the IDs of the SQLitePivot, not the SQLiteModel being returned (a Tag).

In comparison, the MySQL ORM returns ids 1, 2, 3 for the first Bite, and 1, 2, 3, 4 for the second Bite. I would expect this to be the case for SQLite as well.

I have tried all of the methods I found to try and generate a Xcode project for my vapor project but every time i use vapor xcode or vapor update it always get's stuck on
Fetching https://github.com/vapor/sqlite.git
I have tried running it on the latest version of ubuntu and mac OS high sierra but neither work.
I am sure that my vapor toolbox and swift are properly installed as I can perfectly build this project and other projects.

vapor xcode log:

Fetching https://github.com/vapor/vapor.git
Fetching https://github.com/vapor/fluent-provider.git
Fetching https://github.com/vapor-tools/vapor-jsonapi.git
Fetching https://github.com/vapor/bcrypt.git
Fetching https://github.com/vapor/core.git
Fetching https://github.com/vapor/console.git
Fetching https://github.com/vapor/crypto.git
Fetching https://github.com/vapor/engine.git
Fetching https://github.com/vapor/node.git
Fetching https://github.com/vapor/json.git
Fetching https://github.com/vapor/multipart.git
Fetching https://github.com/vapor/routing.git
Fetching https://github.com/vapor/random.git
Fetching https://github.com/vapor/bits.git
Fetching https://github.com/vapor/debugging.git
Fetching https://github.com/vapor/ctls.git
Fetching https://github.com/vapor/tls.git
Fetching https://github.com/vapor/sockets.git
Fetching https://github.com/vapor/fluent.git
Fetching https://github.com/vapor/sqlite.git

I have also tried to create a new project and build that but it still get's stuck on sqlite.

Question:

I was just wondering if the Errors defined here should be more descriptive. For example if there is a constraint error like UNIQUE etc. The execute error feels ambiguous from a client's perspective.

Vapor 4 cannot decode dates.

Steps to reproduce

1. Create an empty project using vapor-beta toolbox and using a SQLite database for testing purposes.

2. Open Todo.swift file and add a field to store a date

Todo model file looks:

import Fluent
import Vapor

final class Todo: Model, Content {
    static let schema = "todos"
    
    @ID(key: .id)
    var id: UUID?

    @Field(key: "title")
    var title: String
    
    @Field(key: "date")
    var date: Date

    init() { }

    init(id: UUID? = nil, title: String, date: Date) {
        self.id = id
        self.title = title
        self.date = date
    }
}

3. Configure the field on the migration file.

import Fluent

struct CreateTodo: Migration {
    func prepare(on database: Database) -> EventLoopFuture<Void> {
        return database.schema("todos")
            .id()
            .field("title", .string, .required)
            .field("date", .date, .required)
            .create()
    }

    func revert(on database: Database) -> EventLoopFuture<Void> {
        return database.schema("todos").delete()
    }
}

4. Run the migrations.

5. Create a new Todo with ISO8601 date.

6. Try to get the todos. It fails.

Expected behavior

The list of todos is returned.

Actual behavior

[ ERROR ] invalid field: date type: Date error: Decoding error: Value of type 'Date' required for key ''.

Environment

• Vapor Framework version: 4.0.0-rc.3.12
• Fluent Framework version: 4.0.0-rc.1
• Fluent-kit Framework version: 4.0.0-rc.1.16
• Fluent-sqlite-driver Framework version: 4.0.0-rc.1.1
• OS version: macOS 10.15.4

Would be really helpful to have this.

I have an SQLiteUUIDPivot and when I try and get it from the database I get the error:

{
    "error": true,
    "reason": "Could not convert to String: 1DA66A7B-91FB-455E-9C70-8FC2C59BC702"
}

Getting an SQLiteUUIDModel works fine

sm.db.create(table: tableName)
            .ifNotExists()
            .column("id", type: .text, .primaryKey(autoIncrement: false), .notNull)

In the example above (simplified from actual case), the presence of a second column constraint (.notNull) leads to bad SQL like

CREATE TABLE IF NOT EXISTS 'TableName'('id' TEXT PRIMARY KEY, NOT NULL) []

Note the extra comma before NOT NULL which causes an error.

Vapor just crashes if I have some data fields with some Arrays. fatalError() doesn’t give a descriptive error.

I wrote a simple User schema with a constraint of a unique email address in the database. In my 'newuser' post method, I am checking the create operation for any database constraint failures. The isConstraintFailure property is set to false, even though the error is SQLiteNIO.SQLiteError.Reason.constraint with a message of UNIQUE constraint failed: users.email.

My function is:

        try User.validate(req)
        let user = try req.content.decode(User.self)
        return user.create(on: req.db)
            .map { user }
            .flatMapError { (error) -> EventLoopFuture<User> in
                if let dbError = error as? DatabaseError, dbError.isConstraintFailure {
                    return req.eventLoop.makeFailedFuture(Abort(.internalServerError, reason: "User Already exists"))
                } else {
                    return User.query(on: req.db)
                        .filter(\.$email, .equal, user.email)
                        .first()
                        .unwrap(or: Abort(.internalServerError))
                }
        }
    }

We would like to use this SQLite driver, however, because of licensing concerns, we cannot proceed until the project has a LICENSE file. Can you add one like you did for the Vapor MySQL driver.

sqlite3_last_insert_rowid describes in its header-doc that it's not thread-safe and might return a different ID if another thread is performing an insert at the same time. When e.g. using a DatabaseConnetionPool, there's only one database (hence only one sqlite handle) with multiple connections accessing it. If two connections now perform an insert, it might happen that the former receives the ID of the latter. This leads to Models having a wrong ID.

Both the classes SQLiteConnectionSource (https://github.com/vapor/sqlite-kit/blob/master/Sources/SQLiteKit/SQLiteConnectionSource.swift#L3-L29) and SQLiteConnection (https://github.com/vapor/sqlite-nio/blob/master/Sources/SQLiteNIO/SQLiteConnection.swift#L60-L64) take a thread pool and a single event loop as init arguments. The core query method (https://github.com/vapor/sqlite-nio/blob/master/Sources/SQLiteNIO/SQLiteConnection.swift#L86-L112) appears to submit a WorkItem to the thread pool which then jumps back to the same eventLoop for every query.

This is not great for multi-threaded workloads where the work is split across multiple event loops (think cores) (e.g. based on incoming HTTP requests) since it means one event loop asymmetrically handles returns from all the DB reads/writes and there is a lot of undesirable event loop switching when e.g. handling an incoming HTTP request.

It looks like it might be a simple change to allow the event loop to be specified per query? (I am also using https://github.com/swift-server/async-http-client which as a comparison allows event loop options (even preference) in execute() calls and whose top-level object, HTTPClient, is instanced with eventLoopGroupProvider (https://github.com/swift-server/async-http-client/blob/master/Sources/AsyncHTTPClient/HTTPClient.swift#L52-L66)).

Interested in your thoughts....

Sample repositopry.
https://github.com/t-ae/fluent-example/tree/sqlite-decoder-bug

I wrote the query that joins persons table two times in MySQL. It works well.
https://github.com/t-ae/fluent-example/blob/sqlite-decoder-bug/Tests/AppTests/AppTests.swift#L61-L113

In SQLite, however, the result [SQLiteColumn: SQLiteData] doesn't have alias but have original one.
Since same table is joined two times, there should be duplicated columns but it is not seen (maybe overriden?).
https://github.com/t-ae/fluent-example/blob/sqlite-decoder-bug/Tests/AppTests/AppTests.swift#L8-L59

In my example, messages table has 4 columns and persons table has 2 columns, so the result of query must have 8 columns.
In this method column count is 8, but the returning columns has duplicate table/name.

It uses sqlite3_column_table_name to get table name.
https://github.com/vapor/sqlite/blob/635ad49b2d6395b63462abffcce58ee38be1e4cc/Sources/SQLite/Database/SQLiteStatement.swift#L151
But documentation comment says:

** ^The names returned are the original un-aliased names of the
** database, table, and column.

Is this dialect definition correct? SQLite supports autoincrement for integer primary keys, so I feel like this should be true and not false.

I ran into this today trying to reuse a Model in a SQLite environment, and was blocked by the 'Autoincrement not supported, skipping' error path.