Support for SQLite Datatypes

[adriencanterot]

Author :

• Adrien Cantérot

Introduction

This PR aims at making SQLite datatypes available to the Fluent's SQLite driver through an enum supporting all available types in SQLite. Plus since a few lines of code were added, I separated the code in 4 files.

[EDIT] The enum has been changed to Node for better integration with Fluent.

Motivation

I don't know if this PR should be considered an enhancement or a fix. SQLite currently gets a String from a row instead of its real type. When converted to a node, it is converted to Node.string even though the data was inserted as an Int or a Double.

It raised a serious issue when wanting to create a Pivot with a Model fetched from a database.
In fact, the id was saved as a Node.string which made it impossible for fluent to retrieve other objects with this id.

Proposed solution

I suggest creating an enum called SQLite.DataType that would represent the four possible SQLite types retrievable which are : Integer, Floating number, String and Null.
In my opinion, it wouldn't be relevant to create a node directly since it's already the driver's job with itsmap function.

[EDIT] See intro

Code Snippet

//Retrieving results from an SQLite row now/
if let resultBar = try database.execute("SELECT * FROM foo WHERE bar = 42").first {
    resultBar.data["bar"] // "42"
    resultBar.data["baz"] // "Life"
    resultBar.data["biz"] // "0.44"
}

//Retrieving results from an SQLite row after.
if let resultBar = try database.execute("SELECT * FROM foo WHERE bar = 42").first {
    resultBar.data["bar"] //edit :  42
    resultBar.data["baz"] // "Life"
    resultBar.data["biz"] // 0.44
}

Impact

The impact would be pretty much inexistant. The one I can think of is from developers who would have fixed this issue by casting their custom properties in their model.

Alternative considered ++

Instead of creating a new type, It would be possible to convert to a Node directly but it would require SQLite to import Node which might not be relevant since it is a very powerful feature for such a small C wrapper. Plus, SQLite driver for Fluent already does that.

[codecov-io]

Current coverage is 69.49% (diff: 74.32%)

Merging #12 into master will increase coverage by 2.82%

@@             master        #12   diff @@
==========================================
  Files             3          5     +2   
  Lines           156        177    +21   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            104        123    +19   
- Misses           52         54     +2   
  Partials          0          0          

Powered by Codecov. Last update 45ca240...734762a

[tanner0101]

I like the changes here a lot, but I think we should use the vapor/node type instead of SQLite.DataType.

This makes it easier to implement the Fluent driver as well as making for a much more robust API for those using this package by itself.

[adriencanterot]

Okay ! you're right, Node provides better integration with Vapor and allows writing more legible code.

[tanner0101]

Looks great! I think we will need to tag this as 2.0.0, though, since the public API will change.

This shouldn't be too much of an issue because we can just upgrade the sqlite-driver to use version 2.0.

[adriencanterot]

Oh sure ! sqlite-driver would update to something like 1.1, or 2.0 as well ?

[tanner0101]

We're probably going to need to tag the driver as 2.0 as well or else it could cause dependency hell for other packages that use 1.*.

I'm going to integrate the new fluent test suite here first in case I need to clean anything up there. Then I will update and tag.