vapor / fluent

Vapor ORM (queries, models, and relations) for NoSQL and SQL databases
https://docs.vapor.codes/4.0/fluent/overview/
MIT License
1.3k stars 171 forks source link

Need easier way to specify field length #439

Closed jseibert closed 6 years ago

jseibert commented 6 years ago

For Fluent to be used in building scalable applications, it needs to be easy to specify and enforce the length of certain fields. Specifically, if I am storing String or Data values of certain sizes, it would ideally be trivial to specify their length without having to write custom migrations or implement custom data types. This should be an easy-to-implement protocol on Model.

Instead, right now, this requires vast amounts of code. For example:

/// Represents a VARCHAR(1024) column.
public struct LongString: MySQLDataConvertible, MySQLColumnDefinitionStaticRepresentable,
                          Codable, Equatable, Hashable, ReflectionDecodable {
    public static func reflectDecoded() throws -> (LongString, LongString) {
        return (LongString(string: "foo"), LongString(string: "bar"))
    }

    /// This TEXT column's string.
    public var string: String

    /// Creates a new `MySQLText`.
    public init(string: String) {
        self.string = string
    }

    public init(from decoder: Decoder) throws {
        var container = try decoder.unkeyedContainer()
        self.string = try container.decode(String.self)
    }

    public func encode(to encoder: Encoder) throws {
        var container = encoder.singleValueContainer()
        try container.encode(self.string)
    }

    /// See `MySQLDataConvertible.convertToMySQLData()`
    public func convertToMySQLData() throws -> MySQLData {
        return MySQLData(string: string)
    }

    /// See `MySQLDataConvertible.convertFromMySQLData()`
    public static func convertFromMySQLData(_ mysqlData: MySQLData) throws -> LongString {
        return try LongString(string: .convertFromMySQLData(mysqlData))
    }

    /// See `MySQLColumnDefinitionStaticRepresentable`
    public static var mySQLColumnDefinition: MySQLColumnDefinition {
        return .varChar(length: 1024)
    }
}
tanner0101 commented 6 years ago

@jseibert I agree. This issue should be taken up on a Fluent driver specifically though (SQLite for example does not have a concept of field lengths). It seems like in this case you are referring to mysql, can we move the issue to vapor/fluent-mysql?

jseibert commented 6 years ago

Is it really driver-specific, or could this be enforced at the Model level on save, and then optionally leveraged by the driver to optimize the database schema?

tanner0101 commented 6 years ago

That's definitely a possibility, though requires some thinking about what Fluent should be. Fluent's goal as stated is to be a framework for building ORMs. This would be an entirely additive feature on top of something like SQLite which has no concept of data lengths which makes me think this would be more appropriate in an additive Fluent validation layer than in Fluent itself. In other words, this feature is higher level than SQLite and shouldn't be a part of the foundational layer to build Fluent SQLite interaction. That said, lengths are supported by many databases, so I wouldn't be against including something like that as an optional protocol in Fluent that those DBs, like MySQL and PSQL, could conform to.

jseibert commented 6 years ago

I don't feel strongly here, so happy to support whichever direction you think is best.

tanner0101 commented 6 years ago

I just made an issue on FluentMySQL for this. I think that is a good place to try a couple of these types out and see how people like them 👍

tanner0101 commented 6 years ago

Recommended way to do this as of now would be a custom migration and the following syntax:

builder.field(for: \.name, type: .varchar(1024))

Going forward Fluent is going to push more toward custom migrations so that the Codable models can stay lightweight. Since there's no good way to specify things like indexes / foreign keys via models, it's better to make a habit of creating migrations. 👍