-
-
Notifications
You must be signed in to change notification settings - Fork 731
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
Query Interface: Join/Create #73
Comments
Hello @cfilipov I don't plan to create tables via Swift, for I don't see much value there, and the amount of work necessary to support SQLite table creation features is better put somewhere else. SQL remains the best tool for the task, IMHO. On the other side, I'm actually working on joins, but could not find a satisfactory API yet. Unlike table creation, helping generating joins can indeed be a boon, and consuming joined rows needs support as well. A natural extension to joins is the support for flat relations such as "has one" and "belongs to" for records. Well, it will ship when it's ready :-) Quick preview: for person in Person.include(birthCountry).fetch(db) {
person.name
person.birthCountry.name
} |
I think there is definitely value in being able to express table creation in Swift for the same reason there is value in having the query builder interface in the first place. If you're using the query interface then you have a bunch of |
I don't quite understand your example. What are the semantics of Side note: I wonder if a SQL DSL should/could be done as separate effort. The query interface seems fairly integrated into the GRDB types but would it be useful to just simply have a Swift DSL that can generate a SQL string and use all the existing GRDB APIs that accept strings? |
@cfilipov, I'll refute each one of your arguments and suggestions below. You'll get a better idea of the nature of GRDB, and you may adjust your suggestions accordingly. They'll be welcome as long as they're aligned with the goals of the library.
Yes. But this kind of typo is quickly noticed, and fixed. I find this argument pretty weak.
You can do that already: just define your constants identifiers somewhere, and use them wherever they suit. No need for library support for that. More generally, I don't want to force users to take this path. GRDB is a bottom-up library, designed for SQL first. High-level APIs build on top of it. There is surely room for improvement, but not the one you describe.
Sorry for the incomplete example :-) // A description of the relation from persons to their birth country:
let birthCountry = ForeignRelation(
to: "countries",
through: ["birthCountryID": "id"], // Joined columns, from left to right
variantName: "birthCountry") // Optional; helps disambiguating
// several relations to the same table
// when appropriate.
// SELECT persons.*, birthCountry.*
// FROM persons
// LEFT JOIN countries birthCountry
// ON birthCountry.id = persons.birthCountryID
Person.include(birthCountry).fetch(db)
Because wrapping the full SQL join syntax into Swift would require the user to define aliases for conflicting columns and tables. It is a chore, and I'd rather ship an ORM-inspired API for joins. Moreover, constructing the joined request allows GRDB to let row adapters grant access to the joined columns in the fetched rows: let request = Person.include(birthCountry)
for row in Row.fetch(db, request) {
row.value(named: "id") // person id
row.variant(named: "birthCountry").value(named: "id") // country id
}
// Hence
for person in request.fetch(db) {
person.name
person.birthCountry.name
} That is the general direction I'm working on these days.
Yep, but the straight forward way in GRDB is SQL, and not a poor quality DSL that's nice on the paper but difficult to remember and use as soon as you have a non-trivial use case (take that, table creation DSL).
I see your point. The separation of the query interface from the core is a background process in my mind. The fetching features of the query interface are already hidden behind the FetchRequest protocol. Another protocol (_SQLExpressible) bubbles up, which allows expression such as
Now that is wrong. Simply generating SQL is not enough (see use of row adapters above). The priority of GRDB is shipping a pragmatic API - there are other more conceptual libraries out there. |
It will be noticed at run time instead of failing at compile time. Compile-time checks are a core strength one enjoys when using Swift. Whenever possible I (and many others) prefer catching an error at compile time vs runtime. And in this case it is possible, so I argue it should be done. I also disagree that this typo is "quickly" noticed. This is only noticed quickly for tables created at app launch, which covers all of your example code, but it doesn't get noticed when a table is created later in the runtime of the app or conditionally. For example: let's say you have several migrations defined: one of the table creation strings from a past migration gets accidentally edited to mis-spell a column (could have happened by editing the wrong string without noticing, or a merge, or refactor, search/replace, etc...). Since the migration has already happened you won't hit this code until you decide to test your app from a fresh state where it will run though all the migrations. Compare this to immediately noticing because of a build failure.
Indeed, that's what I'm doing now: extension Entry.Table {
static func create(db: Database) throws {
try db.execute(
"CREATE TABLE \(Entry.Table.name) (" +
"\(Entry.Column.id.name) integer PRIMARY KEY AUTOINCREMENT NOT NULL," +
"\(Entry.Column.workoutID.name) integer" +
"\(Entry.Column.exerciseID.name) integer NOT NULL" +
");")
}
} But this is pretty tedious, and also, not as type safe. I can use any string for a column, including a string that does not represent a column, whereas a type-safe solution would force one to use a By the way, did you notice that I forgot comma in my example above (near "workoutID")? This is an easy mistake to make, and easy to miss and it won't be noticed until runtime, but if I had a Swift DSL to build this table creation statement then this typo wouldn't be possible.
I definitely don't suggest forcing users into this path. I would expect the SQL string interface to still exist in its current form, in the same way that you are free to choose between the query interface and the SQL string interface today for queries. My suggestion is to provide a type-safe alternative to table creation, but the string APIs would still be the default. I understand your position that there are more important things you would rather focus on, but I strongly disagree that type-safe table creation is not valuable in general (given enough time to tackle it). Type-safe compile time checks are better than stringly-typed API, always. For my project I have been using SQLite.swift up until now. I really liked how my entire schema, all queries and procedures, and even migrations were done in Swift. When I made breaking schema changes, the project failed to compile, when it compiled it usually meant that things will work. That said, SQLite.swift has been kind of a pain. This is subjective: but I find its API to be obtuse. I find GRDB's I also found myself writing certain protocols for SQLite.swift to reduce the amount of boilerplate. I soon discovered this project and realized I had basically implemented the equivalent of Migrating my code to GRDB resulted in less boilerplate, less code overall, and better design and organization. I'm much happier with using GRDB so far, but I really miss having a more complete SQL builder interface. I understand one of your philosophies with GRDB was to provide a Swift library for those who prefer to craft SQL and want its power with the convenience of a higher level library. And I feel that you have achieved that. But I don't think that your philosophy is in conflict with also providing the SQL DSL on top of that like you have in other areas already. |
Let me elaborate on two problems that are latent in the way you look at the topic. The first problem is an application maintenance problem. The second problem lies in the weak definition of "type safety". We'll see that if there is room for improvement in order to lower the dependency on "string programming", I don't think it is the one you champion, and today I really think that GRDB is in better shape when it does not address the problem at all, as it is today, than it would be if it would address it in a bad way. The maintenance problem Your The problem is: putting the table creation statements in classes doesn't fit well with database schema changes and migrations. The How do you cope with migrations when you apply this pattern?
The points above show that the Now the fate of the As a conclusion: should GRDB push for Swift columns and table name constants, it would maybe help, but not as well as you try to put it. One of the core design decisions of GRDB is that the library avoids frictions at all costs whenever the application code base evolves, increases its complexity, enters the non-trivial zone. I think that the delight you describe (thanks :-)) when you use GRDB is a direct consequence of that: picking the best GRDB API for your use case and changing your mind later is usually painless. So if I admit that the problem you describe exists, the solution you propose is too rigid. And I don't have a better one. I'd thus rather put some burden on application code, even if an unfortunate consequence is that applications take debatable design decisions. I feel sorry, but that's how experience grows. You may have guessed that I personnally feel very happy with string programming: once a migration has been written and checked, I never think about it again, and I quite like that. Type safety Quite a subject. So hyped, so badly understood. SQLite.swift is the champion of type safety, isn't it? Well, I think that the way SQLite.swift tackles the topic is broken beyond any hope. I have to go know, but I'll come back with a extended rationale :-) |
BTW - I'm not trying to close the discussion: I just want to say that the subject is more delicate that it looks at first sight. GRDB is flexible, maybe too flexible, but it allows applications to use it in the way they want. I'm myself balanced between the desire to avoid boilerplate and runtime checks, and the ability to go as raw as I need whenever relevant. So far, the ability to go raw prevails, that's true, because I feel like all other opinionated high-level APIs I've seen fall in three categories: toys, locks, and bombs. Toys are limited, locks reveal unsuitable long after you've invested too much time with them to leave, and bombs punish you hard when you misuse them. There are many toys out there, SQLite.swift is both a lock and a bomb, Core Data is a bomb. There's a reason GRDB exists. |
Regarding your comment on table creation. I agree, that approach would be limiting for the reasons you mentioned. My code snippets purpose was to illustrate the pitfalls of building SQL This is a strategy I employed in my app which currently uses SQLite.swift, but I was able to use the same design with GRDB as well (but haven't pushed the code yet). I'm not suggesting people should do this, by the way. This is a personal project where I have the luxury of experimenting with weird ideas I otherwise couldn't. But I hope you will agree this weird idea seems to work pretty well. At the very least it might be on the right track towards a better idea.
So using this trick I can now group my Swift SQL schema into enums for each version of the Schema (each
Each
Somewhere, globally in the app, I define the current schema:
The actual
Then, I associate the model with its schema:
Now, here's how I conform to the GRDB protocols:
And some usage in queries:
Note that if I don't care about mapping rows to a struct, I can just use the Now let's say I change the schema. I create a new enum with new table definitions, and typealias the parts of the previous schema that have not changed. All the table creation and migration is handled in the self-contained schema. I then change the global alias In summary: I have all my SQL tables and columns namespaced into enums for each migration. And the models just typealias to their corresponding parts of the current schema, which is itself also a typealias to the latest schema. This ensures I have the Swift representation of the schema for each migration, so migrations can be done in pure swift (when using SQLite.swift). It's not perfect, but this approach has been very useful for me, it's worked well and has caught many issues at compile time that otherwise would have only been detected at runtime (if I'm lucky). It's won't catch everything at compile time, but it catches some of the easiest mistakes one can make and works extremely well so far. |
Wow, @cfilipov, that's quite an involved setup :-) Listen: I still have to come up with something with joins. Today I'm still wondering how to deal with automatic table aliasing, precisely how to let the user express WHERE and ON conditions on aliased tables without referring to them via strings - that's tricky, and I'm not even sure I'll eventually succeed properly. I hope this process will help me clarifying my ideas on the string vs. compiler constants. I could not manage to write about type safety as I promised - to be short:
Remember: GRBD is a tool, not a proof-of-concept. This is paramount. |
I think your first point about the mismatch of trying to tack on a static type system to SQLite's weak type system is quite good and I understand that. As far as the second point, yes the compile errors can get very hard to understand. It seems you prefer clear runtime errors to unclear compile time errors. I disagree with this but respect your decision to drive your project in the direction you feel is best. Given all this, why even both to implement a Swift query building interface in the first place? Anyway, I'll stop bugging you about this topic, I think we beat it to death already. And as you said, there are higher priorities, like getting ready for Swift 3, you're on that, right? ;). |
Because some people fell uncomfortable with SQL, that's it!
You have seen the Swift3 branch already, didn't you? |
I saw that branch. I was just playfully reminding you that's a more important priority so you should stop paying attention my silly issues like this and get back to finishing that Swift 3 branch. One of the reasons I picked up this library in the first place was because I'm not seeing any indication from SQLite.swift that he's going to migrate any time soon. Thanks for the great work and taking the time to debate the SQL builder interface, looking forward to the future iterations. I still hope you change your mind in the future, but for now I think it's time to close this issue. |
FYI @cfilipov, the Swift3 branch is already in good shape, and follows swift evolution pretty closely. It should even be delicate migrating your GRDB code from Swift2 to Swift3, since a lot of APIs have changed to better reflect the new API guidelines. If you have a project that uses GRDB and that you consider migrating to Swift3, I advise you to migrate now, since each incremental update to the Swift language will be much easier after the initial migration. And you'll be ready when Swift3 is officially shipped. |
Another option is always creating the latest version of table in create() method. Update logic can look like this: if database is not created yet, .create() all of the tables and set this version as latest, otherwise apply migrations. Is this more error-prone? |
Sorry to resurrect this thread but I'd like ask: If I were to submit a pull request that introduces a query builder interface for table/view creation and joins would you consider accepting it? |
@cfilipov Welcome back :-) Let's avoid spending time for nothing: before submitting a PR, don't forget to make a general proposal with a few code snippets that illustrate the usage of the DSL you envision. Suggested use cases (1 is unavoidable to my eyes, 2 is nice to have and should have room): -- (1) regular table creation (2) temporary tables
-- Out of scope: tables without rowid, schema names, create if exists,
-- virtual tables, create table as select...
CREATE TABLE foos (
-- (1) rowID primary key
id INTEGER PRIMARY KEY,
-- (1) The four main SQLite affinities, (2) numeric affinity
text TEXT,
int INTEGER,
data BLOB,
double DOUBLE,
numeric NUMERIC,
-- (1) NOT NULL, (1) DEFAULT, (2) COLLATE
foo TEXT NOT NULL DEFAULT 'foo' COLLATE nocase
-- (1) references with full cascade/restrict/etc actions
barID INTEGER NOT NULL REFERENCES bars(id) ON DELETE CASCADE ON UPDATE CASCADE
-- (2) multi-column references
x INTEGER,
y INTEGER,
FOREIGN KEY(x, y) REFERENCES bars(a, b)
-- (1) multi-column primary key
PRIMARY KEY (a, b)
)
-- (1) indexes, eventually unique, (2) eventually covering
CREATE INDEX fooOnBaz foo(baz)
-- (1) ALTER TABLE ... ADD COLUMN
ALTER TABLE foos ADD COLUMN bar INTEGER About the type of columns: I already see that the |
@cfilipov What I care about here is that falling back to SQL should not be shocking. Let me try to explain: As soon as you use a high-level DSL, having sometimes to go back to SQL looks odd. This oddity yields subjective feelings like:
Those feelings can't be 100% avoided. And there are three victims whose subjective quality suffers: applications, developers, and GRDB itself. So I wish the table creation DSL would be expressive, consistent, and comprehensive enough to leave SQL to the really special migrations. |
@cfilipov Another point that deserves a little place in our mind: https://github.com/groue/GRDB.swift#advanced-database-schema-changes Now I guess you know why I want us to discuss before we code :-) |
@cfilipov You launched me, I can't stop:
Procedural is: "do this, then do that", and supported by the current DatabaseMigrator. Descriptive is "make this happen", and is not supported today. Descriptive is harder: if you plan to have a |
@cfilipov Oh, I forgot about confict resolution clauses! Let's put them in the "nice to have". GRDB has a weakness here: conflict resolution clauses make it difficult to know whether an INSERT statement has inserted a row or not - and |
I've never used the
The difficult thing about making the DSL completely declarative is that you end up too magical, too far removed from the SQL and it starts to look more like an ORM, and I believe neither one of us wants a full-blown ORM. My goal was to have a little "magic" as possible in the DSL. PrerequisitesBefore showing you examples I think it's worth taking the time to briefly share some pseudo code of how the examples can be implemented. BuildersI envision a builder pattern approach where each builder object is responsible for building a subset of the public protocol BuilderType {
func build() throws -> String
}
public protocol CreateBuilder : BuilderType { } Each builder implements the This pattern also makes it somewhat easy to expand the DSL grammar over time to more completely support SQLite's features. For example, adding Some builders can also be re-used, like the The following pseudo code roughly outlines what it would look like for a This code is just to help illustrate the example usage, I don't want to get too far into the details of the builder until we settle on usage (though of course they are related and we may end up needing to discuss them in more detail). public struct CreateTableBuilder: CreateBuilder {
var name: String
var temp: Bool = false
var ifNotExists: Bool = false
public func build() throws -> String {
var parts = ["CREATE"]
if temp {
parts.append("TEMP")
}
parts.append("TABLE")
if ifNotExists {
parts.append("IF NOT EXISTS")
}
return parts.joined(separator: " ")
}
public func columns(_ columns: [ColumnType]) -> SelectFromBuilder {
return CreateTableColumnsBuilder(prefix: self, columns: columns)
}
}
public struct CreateTableColumnsBuilder: CreateBuilder {
let prefix: CreateTableBuilder
var columns: [ColumnType]
public func build() throws -> String {
precondition(columns.count > 0)
let columnDefs = columns.map { try $0.build() }.joined(separator: ", ")
return try [prefix.build(), "(", columnDefs, ")"].joined(separator: " ")
}
public func foreignKeys(_ columns: [ColumnType]) -> CreateForeignKeyClauseBuilder {
...
}
}
public struct CreateTableConstraintBuilder: CreateBuilder {
...
} Column TypeI also changed GRDB's Example UsageNow let's get into the usage. I am going to show how the builder interface for tables could look, and how it is flexible enough to let the developer choose how they organize their code. Simple ExampleThe following code demonstrates how you would create a table using this builder interface. This is almost a 1-1 transcription of the actual SQL, anyone familiar with SQL should find it fairly readable. Let's see a simple table first: TableNamed("bar")
.create(ifNotExists: true)
.columns(
Column<Int64>("id")
.primaryKey(autoIncrement: true),
Column<String>("text")) Now a more complete example: TableNamed("foo")
.create(ifNotExists: true)
.columns( // var args
Column<Int64>("id")
.primaryKey(autoIncrement: true), // autoIncrement param is optional
Column<Int64?>("int"), // nullable
Column<String>("text")
.unique(onConflict: .rollback)
.default(0)
.collate(.noCase),
Column<Data>("data"),
Column<Double>("double"),
Column<Int64>("bar_id")
.onNull(.ignore) // only available on Column<T> where T is non-optional
.references(
table: Bar, // or Bar.databaseTableName()
onDelete: .cascade, // [optional]
onUpdate: .cascade // [optional]
deferrable: true, // [optional] default: false
columns: Bar.Column.id)) // var args Column VariablesThe problem with the previous code is that we aren't able to re-use those columns in our queries. We would need to keep copying static let id: Column<Int64> = "id"
static let int: Column<Int64?> = "int"
static let text: Column<String> = "text"
static let data: Column<Data> = "data"
static let double: Column<Double> = "double"
static let bar: Column<Int64> = "bar_id"
TableNamed("foo")
.create(ifNotExists: true)
.columns(
id
.primaryKey(autoIncrement: true),
int,
text
.unique(onConflict: .rollback)
.default(0)
.collate(.noCase),
data,
double,
bar
.onNull(.ignore)
.references(
table: Bar,
onDelete: .cascade,
onUpdate: .cascade
deferrable: true,default: false
columns: Bar.Column.id)) The same for static let id: Column<Int64> = "id"
static let text: Column<String> = "text"
TableNamed("bar")
.create(ifNotExists: true)
.columns(
id.primaryKey(autoIncrement: true),
text) Grouping Column VariablesThose free-floating variables may conflict with each other (both tables define enum Foo {
static let table: TableNamed = "foo"
enum Column {
static let id: Column<Int64> = "id"
static let int: Column<Int64?> = "int"
static let text: Column<String> = "text"
static let data: Column<Data> = "data"
static let double: Column<Double> = "double"
static let bar: Column<Int64> = "bar_id"
}
}
enum Bar {
static let table: TableNamed = "bar"
enum Column {
static let id: Column<Int64> = "id"
static let text: Column<String> = "text"
}
} This isn't part of GRDB or enforced by the builder interface. This is just one way someone might choose to organize their column definitions, and the builder interface works just fine with this approach: Foo.table
.create(ifNotExists: true)
.columns(
Foo.Column.id
.primaryKey(autoIncrement: true),
Foo.Column.int,
Foo.Column.text
.unique(onConflict: .rollback)
.default(0)
.collate(.noCase),
Foo.Column.data,
Foo.Column.double,
Foo.Column.bar
.onNull(.ignore)
.references(
table: Bar,
onDelete: .cascade,
onUpdate: .cascade
deferrable: true,
columns: Bar.Column.id))
Bar.table
.create(ifNotExists: true)
.columns(
Bar.Column.id.primaryKey(autoIncrement: true),
Bar.Column.text) Defining Column Constraints EarlyAlternatively, we can define column constraints at the time we define the columns, rather than at table creation: enum Foo {
static let table: TableNamed = "foo"
enum Column {
static let id = Column<Int64>("id")
.primaryKey(autoIncrement: true)
static let int = Column<Int64?>("int")
static let text = Column<String>("text")
.unique(onConflict: .rollback)
.default(0)
.collate(.noCase)
static let data = Column<Data>("data")
static let double = Column<Double>("double")
static let bar = Column<Int64>("bar_id")
.onNull(.ignore)
.references(
table: Bar,
onDelete: .cascade,
onUpdate: .cascade
deferrable: true,
columns: Bar.Column.id)
}
}
struct Bar {
static let table: TableNamed = "bar"
enum Column {
static let id: Column<Int64> = "id"
static let text: Column<String> = "text"
}
} Now the table creation would look like this: Foo.table
.create(ifNotExists: true)
.columns(
Foo.Column.id,
Foo.Column.int,
Foo.Column.text,
Foo.Column.data,
Foo.Column.double,
Foo.Column.bar)
Bar.table
.create(ifNotExists: true)
.columns(
Bar.Column.id,
Bar.Column.text) This approach makes the column definitions slightly less readable, but provides more context where is more likely to be seen. When you are building a query you might jump to the definition of Table ConstraintsAn example using table constraints: Baz.table
.create(ifNotExists: true)
.columns(Baz.Column.someID, Baz.Column.otherID, Baz.Column.foo)
.primaryKey(Baz.Column.someID, Baz.Column.otherID)
.foreignKeys(Baz.Column.foo)
.references(table: Foo, columns: Baz.Column.foo) Making Columns Available on Your ModelIf you want to have all your column definitions available as statics on your actual model object struct Bar: TableMapping {
var id: Int64
var text: String
static func databaseTableName() -> String {
return "bar"
}
enum Column {
static let id: Column<Int64> = "id"
static let text: Column<String> = "text"
}
}
Bar
.create(ifNotExists: true)
.columns(
Bar.Column.id,
Bar.Column.text)
let row = Bar.filter(Bar.Column.id == 42).fetchOne(db) Migrating would involve refactoring the code to rename MigrationsMigrations are still handled procedurally. The SQL statements themselves are declarative, but the actions of creating a table or migrating a database are procedural. I think this is a nice compromise between procedural and declarative. As you said, going all the way and making the table fully declarative would probably require generating SQL diff for migrations which I find too automatic and a lot of work to implement correctly. I just want a way to write SQL without gluing strings together manually, but I still want to essentially write SQL. Final ThoughtsImplicit TablesIt might be valuable to define columns with their tables: static let table: TableNamed = "foo"
static let id = Column<Int64>(name: "id", table: table)
static let text = Column<String>(name: "text", table: table) This is convenient later on when you want to do something like join a table and need to qualify the column with the table name. Without this, it would be up to the developer to remember to do this when they build the query. This wouldn't be such a big deal if I could find a more elegant syntax for doing that. It would also make foreign key references when creating a table simpler. You would only have to provide a The trade off here is the extra verbosity when defining your columns, but maybe it's worth it. Expressions and StatementsTo support things like public protocol BuilderType {
func build(_ arguments: StatementArguments) throws -> String
} This allows us to avoid the Columns as a param to Create()The Foo.table
.create(ifNotExists: true, columns:
Foo.Column.id,
Foo.Column.int,
Foo.Column.text,
Foo.Column.data,
Foo.Column.double,
Foo.Column.bar) Params vs MethodsAs discussed in the previous section, there are many other places where one could just as easily implement part of the builder as a parameter to a builder method or as a separate builder method. I'm interested in which ones make the most sense. |
Many thanks @cfilipov. Please let me have a detailed reading. |
Three points:
The builder pattern has indeed many advantages:
It also disadvantages, because of the abundance of ad-hoc types it requires:
I appreciate that your sample code follows closely the SQL syntax.
SQLColumn is part of the query interface, and the query interface is untyped. I see a conflict in your change and I recommend that you revise your copy. I'm against a typed query interface. It generates bad compiler errors that are hard to understand (StackOverflow is full of questions about SQLite.swift because of that). And it is unable to talk fluently to the weak typing of SQLite. That's why in GRDB, types are pulled by the user: let row = Row.fetchOne(db, "SELECT '2016-06-30'")!
let string: String = row.value(atIndex: 0) // "2016-06-30"
let date: Date = row.value(atIndex: 0) // Date
self.foo = row.value(atIndex: 0) // depends on the type of property SQLColumn and the query interface extend this pattern, not the opposite.
Here I don't follow you. To make your code nice-looking, you have to build a cathedral. You have already talked about it above. Let me be clear: this pattern is much too much demanding, is representative of you current way of addressing your needs, and can be easily criticized. It is not a reliable and recommendable ground. Thus it is not a selling point for your table creation API. Actually, it's the opposite. Maybe another approach would be to try to write documentation, instead of trying to convince me how brilliant the DSL is. Try to see it from the point of view of a regular developer, and help him instead of telling him how he should code. If the tool is inspiring, imaginative readers will be inspired. |
There's more advantages:
As for disadvantages:
I have to agree this is a good argument against them. You could hide the builders by making them private and having a wrapper type be returned to each builder, but then chaining would not longer be type-safe and it would be easier to construct incorrect statements. This would also still leak the wrapper. That said. I think it's OK the have this be visible to the public interface. Perhaps there is part of the SQL grammar you don't want to support right now. Someone can extend the builders to add that support themselves.
I think the abundance of types makes things easier to follow. A well-structured graph is much more clear than trying to figure out what a long set of instructions are tying to do. Your query code is the opposite of the builder. It is a single class responsible to building the query procedurally. I believe if it were a builder it would be easier to implement and reason about joins (and I don't just believe that. I actually re-wrote your query builder as a set of separate builders and saw several advantages as I mentioned earlier. I think the trade off was worth it).
I believe this to be a very weak argument. It's easy to make any code unmanageable. If it's not appropriate to add a method, don't merge it. If the DSL is becoming too wide please tell me what parts you wish to be removed.
I don't understand what the problem is with more types. What practical problem do they cause? Some people are deterred by declaring too many types, and often for no reason other than "there's too many". If you can solve your problem more clearly and with better correctness by using more types then declaring more types is appropriate. If you are writing longer functions which do more and are more complex just to avoid declaring more types what are you really gaining? Will you accept a PR with a builder? Are you prompting for more debate? Will you suggest an alternative? I'm not sure how to proceed here.
I never proposed a typed query interface. The typed column does not change how queries are done, the type is only used for building tables. public protocol ColumnType: _SpecificSQLExpressible {
var name: String { get }
}
public struct Column<DataType>: ColumnType {
public typealias ColumnType = DataType
public let name: String
public let table: Table?
...
} Throughout the query code you would use
This is the pattern you are using in your own example code. If you provide the user with the ability to represent columns in Swift as you have, they will need a way to organize that code and use it in their queries. And as you said, if you give them a way to use column variables in their table creation code they will need a way to use those variables in migrations. You cannot ignore the way the code will be used when designing the API. I have provided several examples of how one could use this API, and you seem to be getting distracted by just one. If you do not like one approach, please advice an alternative. I'm not in love with this pattern, it's just the only one I have found that works. I would love to hear your solutions for building sql in Swift while also handling migrations in Swift. It seems you have been distracted by certain parts of this conversation and failed to address the actual API itself. You haven't provided any feedback on the Let me also be clear, I am not begging you to adopt my code. I got what I need working and I offer my free time to modify it in such a way that you might accept it as a PR. While we often disagree on many things, I am still delighted by the code you have produced, so I know that if I collaborate with you it could result in much better design than if I were to keep my own private fork as I have been so far. If you don't find value in any of this I am happy to back off and leave this as-is. |
oops. I hit "Close and comment" instead of just "Comment" |
A couple changes to the API you proposed: it seems the most idiomatic way to do this in Swift 3 is to use the first parameter label to differentiate the different create functions. So instead of Schema.create(table: "foo", ...)
Schema.create(index: "foo_index", ...)
Schema.create(virtualTable: "bar", ...)
Schema.create(view: "foo_view", ...)
Schema.create(trigger: "foo_on_insert", ...) Dropping tables: Schema.drop(table: "foo", ...)
Schema.drop(index: "foo_index", ...)
Schema.drop(virtualTable: "bar", ...)
Schema.drop(view: "foo_view", ...)
Schema.drop(trigger: "foo_on_insert", ...) Alter: Schema.alter(table: "foo")
.add(Column("new_col", affinity: .integer)) // no need for `column:` label because the type can be inferred
Schema.alter(table: "foo").rename(to: "foo2") Full create example: Schema.create(table: "foo", ifNotExists: true, columns:
Column("id", affinity: .integer, notNull: true)
.primaryKey(autoIncrement: true),
Column("int", affinity: .integer),
Column("text", affinity: .string)
.unique(onConflict: .rollback)
.defaults(0) // unfortunately, `default` is a reserved keyword.
.collate(.noCase),
Column("data", affinity: .data),
Column("double", affinity: .double),
Column("bar_id", affinity: .integer)
.onNull(.ignore)
.references(
table: "bar",
onDelete: .cascade,
onUpdate: .cascade
deferrable: true,
columns: Column("id"))) The affinity of a column is an optional part of the grammar, and defaults to Even more descriptive, but also more verbose, using a Schema.alter(table: "foo")
.add(Column(name: "new_col", affinity: .integer)) Also might make sense to name migrator.registerMigration("v2") { db in
db.execute(
SQL.alter(table: "foo")
.add(Column(name: "new_col", affinity: .integer)))
} Alternately migrator.registerMigration("v2") { db in
SQL(db).alter(table: "foo")
.add(Column(name: "new_col", affinity: .integer))
} |
The examples bring up the question of how The example you used, which is a mix of procedural and declarative by using a closure, would solve this by putting the root column builder behind a method: SQL(db).create(table: "foo") { table in
table.add(column: "id", affinity: .integer, notNull: true)
.primaryKey(autoIncrement: true)
table.add(column: "text", affinity: .string)
.unique(onConflict: .rollback)
.defaults(0) // unfortunately, `default` is a reserved keyword.
.collate(.noCase)
} This makes the builders a bit more complicated, but I haven't fully thought out how this implementation would look so maybe it's not that complicated. It might tempting to do things like add an index to the table inside SQL(db).create(index: "foo_idx", on: "foo"...)... Here we're being stringly again and might mis-type "foo". It might be useful to return a "table" instance and use that to create idexes etc... let foo = SQL(db).create(table: "foo"...)...
foo.create(index: "foo_idx"...)...
foo.alter(table: "foo"...)...
foo.drop() The let foo = SQL.table(named: "foo") I would prefer to avoid the closure approach and just put the column definitions behind a builder method like this. Here there would be a migrator.registerMigration("v2") { db in
db.execute(SQL
.create(table: "foo", ifNotExists: true)
.column(name: "id", affinity: .integer, notNull: true)
.primaryKey(autoIncrement: true)
.column(name: "int", affinity: .integer)
.column(name: "text", affinity: .string)
.unique(onConflict: .rollback)
.defaults(0)
.collate(.noCase)
.column(name: "data", affinity: .data)
.column(name: "double", affinity: .double)
.column(name: "bar_id", affinity: .integer)
.onNull(.ignore)
.references(
table: "bar",
onDelete: .cascade,
onUpdate: .cascade
deferrable: true,
columns: "id"))
} I think the above snippet is the cleanest and also most straight forward to implement so far. |
My apologies for the bombardment of comments. I have been iterating on this idea a lot. I considered deleting my previous comments to make it easier to follow the discussion, but it might be helpful to see how I arrived at the current proposal so I will leave them. Feel free to skip down this this comment if it's too much to read. I moved most of the builder methods into labeled parameters. Builder methods work better when there are mutually exclusive options in the grammar, but in most cases I was able to work around that with enums. This might be possible in all cases but so far I have not run into a case where it didn't work. Table constraints (not shown) would still be added via chained builder methods. migrator.registerMigration("v2") { db in
db.execute(SQL.create(table: "foo", ifNotExists: true)
.column(
name: "id",
affinity: .integer,
notNull: true, // default is `true`
primaryKey: true),
.column(name: "int", affinity: .integer)
.column(
name: "text",
affinity: .text,
unique: .rollback, // can also be `true`/`false` or other enum values, `false` is default.
default: 0, // `default` *can* be used as an argument label, but the xcode highlighter mistakenly marks it red like other reserved words
collate: .noCase)
.column(name: "data", affinity: .data)
.column(name: "double", affinity: .double)
.column(
name: "bar_id",
affinity: .integer,
onNull: .ignore,
references: SQL.foreignKey(
table: "bar",
onDelete: .cascade,
onUpdate: .cascade
deferrable: true,
columns: "id"))
} I think that this is overall a nicer syntax than the previous examples up to this point, except for two potential problems:
The SQL.create(name: "foo", ifNotExists: true)
.column(
name: "id",
primaryKey: true, // non-auto-incrementing primary key
affinity: .integer,
notNull: true) To define conflict resolution: SQL.create(name: "foo", ifNotExists: true)
.column(
name: "id",
primaryKey: .onConflict(.ignore), // non-auto-incrementing primary key
affinity: .integer,
notNull: true) To specify auto-incrementing: SQL.create(name: "foo", ifNotExists: true)
.column(
name: "id",
primaryKey: .autoIncrement(order: .desc, onConflict: .abort),
affinity: .integer,
notNull: true) Unfortunately, because I used an enum with associated values, you are forced to specify Here's another example, this time with table constraints: migrator.registerMigration("v2") { db in
let fooId = SQLColumn("foo_id")
let bazId = SQLColumn("baz_id")
let a = SQLColumn("a")
let b = SQLColumn("b")
let c = SQLColumn("c")
let createBar = SQL.create(table: "bar", ifNotExists: true)
.column(name: fooId, affinity: .integer)
.column(name: bazId, affinity: .integer)
.column(name: a, affinity: .integer)
.column(name: b, affinity: .integer)
.column(name: c, affinity: .integer)
.unique(columns: fooId, bazId, onConflict: .rollback) // this is legal in Swift, but would you prefer to have these parameters reversed? (I think it reads better in this order though)
.primaryKey(columns: a, b, c, onConflict: .rollback) // runtime error if multiple `primaryKey()` used
.foreignKey(
columns: fooId,
onConflict: .rollback,
references: SQL.foreignKey(
table: "foo",
columns: "id"))
.foreignKey(
columns: bazId,
onConflict: .rollback,
references: SQL.foreignKey(
table: "baz",
columns: "id"))
.check(a != b)
.check(b != c)
db.execute(createBar)
} Above you see how the shorter |
@cfilipov I think we're on the right track now :-). Don't code yet, because I want to discuss the fluent method chaining (although I bet on you this time), a few other minor points like the intermediate SQL generation step, and I'd also like to see if we have room for future remove/alter column migrations (the ones that SQLite does not support easily, and require foreign key disabling - they might (I don't know yet) have an impact on the API). |
I'm happy to hear that our ideas are now converging @groue. Don't worry, I'm not trying to waste time writing code that won't get accepted, but I did write the bare minimum code (highly stubbed) needed to test out some of the ideas to make sure I wasn't getting away with impossible syntax. Edit: By the way, my understanding is that you cannot drop or rename a column in SQLite. You have to create a new table with a temp name, migrate the data, then drop the old table and rename the new one. |
(Very long and detailed answer)
Yes. To put it in another way: it must be possible to avoid any change (up to the git diff). People can do otherwise, it won't be my problem.
This looks like a benefit, but I stand by my position: it is an anti-pattern that threats application maintenance over time. When the code doesn't describe the database state, we have a big problem. What I call your "cathedral" of versioned enums and types is an inflated answer to a real problem. I stand that the problem should be avoided, not cured.
I feel sorry when I see people dive in the current trend for type safety and forget about higher level development priorities. It's a tool, not more.
All right.
OK we'll see that in the details of the DSL.
GRDB uses the Configuration type for that: var config = Configuration()
config.readonly = true
config.foreignKeysEnabled = true // The default is already true
config.trace = { print($0) } // Prints all SQL statements
let dbQueue = try DatabaseQueue(
path: "/path/to/database.sqlite",
configuration: config)
True :-)
OK let's talk about the definition of the types of columns. What are our options?
I'd vote for SQL types.
Wouldn't BLOB be the best affinity for this case? Quoting SQLite doc:
Nevertheless, thanks for this really surprising suggestion. I foresee documentation problems, because I don't see myself starting by this case (it doesn't look very serious at first sight), and by the time users have understood typed columns, they won't be much impressed by this unexpected shortcut. What do you think?
OK let me try - first draft, heavily inspired by Active Record migrations and your own sample codes, with semi-realistic models: db.schema.create(table: "persons") { t in
t.column("name", .text, default: "Anonymous") // defaults, defaulting?
t.column("birthDate", .date)
t.column("email", .text, unique: true, collate: .nocase)
}
db.schema.create(table: "countries", primaryKey: "isoCode") { t in
t.column("isoCode", .text)
t.column("name", .text, null: false)
}
db.schema.create(table: "citizenships", primaryKey: ["personId", "countryIsoCode"]) { t in
t.column("personId", .integer, null: false)
.references("persons", onDelete: .cascade, onUpdate: .cascade)
t.column("countryIsoCode", .text, null: false)
.references("countries", onDelete: .cascade, onUpdate: .cascade)
}
db.schema.create(table: "passports") { t in
t.column("personId", .integer, null: false)
t.column("countryIsoCode", .text, null: false)
t.column("issueDate", .datetime)
t.references("citizenships", from: ["personId", "countryIsoCode"])
} A few notes on the attempt above:
Table modification: db.schema.alter(table: "persons") { t in
t.addColumn("lastSyncDate", .datetime)
t.removeColumn("birthDate")
// add not null constraint
t.changeColumn("email", .text, unique: true, null: false, collate: .nocase)
} Removing columns and changing columns is for the future. But grouping the altering statements is important for the DSL to be able to generate the required SQL. See https://github.com/groue/GRDB.swift#advanced-database-schema-changes for an example. A problem is that Indexes: db.schema.create(index: "personsOnEmail", on: "persons", columns: ["email"], unique: true) I'd like to make the index name optional: db.schema.create(indexOn: "persons", columns: ["email"], unique: true) OK, back to you:
Yep. That's why I have tried to get rid of any visible value type in the migration code.
Exactly. Our sample codes are very close here.
Let's help the API users before ourselves :-)
Looks sensible.
I agree that the string variables can look quite ad-hoc: let personsTable, name, birthDate, email = "persons", "name", "birthDate", "email"
db.schema.create(table: personsTable) { t in
t.column(name, .text)
t.column(birthDate, .date)
t.column(email, .text, unique: true, collate: .nocase)
}
db.schema.create(indexOn: personsTable, columns: [email], unique: true) I admit that the avoidance of value types has a cost.
Let me try the same table: migrator.registerMigration("v2") { db in
db.schema.create(table: "foo", ifNotExists: true) { t in
t.column("int", .integer)
t.column("text", .string, default: "0", collate: .noCase)
.unique(onConflict: .rollback)
t.column("data", .blob)
t.column("double", .double)
t.column("bar_id", .integer)
.notNull(onConflict: .ignore)
.references("bar", onDelete: .cascade, onUpdate: .cascade, deferrable: true)
}
} The conflict clauses on unique and not null require a builder extension just like in your sample code. Thoughts?
You're totally welcome.
Hu hu you did just touch the limits of a pattern. I'm happy that you can feel when such problems arise.
I agree it requires a litle juggling with builders and arguments until the API looks correct.
I'm not sure my solution for primary keys is the best either (default "id INTEGER PRIMARY KEY", and listing the primary key columns in the
It's been a while I wanted to tell you that the "auto-incrementing" word should not be used in our API for "INTEGER PRIMARY KEY" columns. SQLite does not recommend the usage of truly auto-incremented columns for performance reasons. Instead, "INTEGER PRIMARY KEY" columns guarantee automatic uniqueness (the most common goal), but do not guarantee incrementation (ids of deleted rows can be reused). Still you remind me I did not handle conflicts and ordering of primary keys in my sample code. Duly recorded.
I'll answer later on table constraints. Conclusion of this message: I have tried to show an alternative. I did not cover as many cases as you, so I'm not sure yet it will stand. Thanks a lot for your involvement here, it is greatly appreciated. |
Side note - a glimpse on future joins. Joins belong to the query realm so they won't be used when creating table references. Yet it would be better if they looked somewhat similar. Let's jump right into a non-trivial join problem, and take a person which has two parents and a birth country: CREATE TABLE persons (
id INTEGER PRIMARY KEY,
name TEXT,
fatherId INTEGER REFERENCES persons(id),
motherId INTEGER REFERENCES persons(id)
birthCountryId INTEGER REFERENCES countries(id),
) Joins in the query interface aim at solving the following use cases:
OK let's dive in. To use joins, you need to declare a relation which tells how to reach a joined table. Starting from persons: // Current API for relation declaration - the one that we'll make akin to
// the reference creation API:
let father = ForeignRelation(named: "father", to: "persons", through: ["fatherId": "id"])
let mother = ForeignRelation(named: "mother", to: "persons", through: ["motherId": "id"])
let country = ForeignRelation(named: "birthCountry", to: "countries", through: ["birthCountryId": "id"]) To perform requests, you also need a starting point on which relations apply. Current limitation: you need a TableMapping type. From the starting point, you can include relations (the columns of the joined tables will be selected), or simply join them (the columns of the joined tables will not be selected): // Give me all persons along with their parents and birth country
let request = Person.include(father, mother, country)
// SELECT persons.*, father.*, mother.*, countries.*
// FROM persons
// LEFT JOIN persons father ON father.id = persons.fatherId
// LEFT JOIN persons mother ON mother.id = persons.motherId
// LEFT JOIN countries birthCountry ON birthCountry.id = persons.birthCountryId
for person in request.fetch(db) {
person.father
person.mother
person.country
} At the row level: for row in Row.fetch(db, request) {
row.value(named: "id") // id of the person
row.variant(named: "father").value(named: "id") // father's id
row.variant(named: "mother").value(named: "id") // mother's id
row.variant(named: "birthCountry").value(named: "id") // country id
} (I'm not happy with the "variant" word here - figuring out the right API for row adapters is bloody hard.) Loading nested relations: let request = Person
.include(
father.include(father, mother),
mother.include(father, mother))
// SELECT persons.*, father0.*, father1.*, mother0.*, mother1.*, father2.*, mother2.*
// FROM persons
// LEFT JOIN persons father0 ON father0.id = persons.fatherId
// LEFT JOIN persons father1 ON father1.id = father0.fatherId
// LEFT JOIN persons mother0 ON mother0.id = father0.motherId
// LEFT JOIN persons mother1 ON mother1.id = persons.motherId
// LEFT JOIN persons father2 ON father2.id = mother1.fatherId
// LEFT JOIN persons mother2 ON mother2.id = mother1.motherId
// LEFT JOIN countries birthCountry ON birthCountry.id =
for row in Row.fetch(db, request) {
// The nesting of row variants follows the nesting of relations:
row // person
row.variant("father") // person's father
row.variant("father").variant("father") // person's father's father
row.variant("father").variant("mother") // person's father's mother
row.variant("mother") // etc.
row.variant("mother").variant("father")
row.variant("mother").variant("mother")
}
// Variant nesting fits well with the consumption of row variants by model types:
for person in request.fetch(db) {
person.father.father
} Joining table without selecting them: // Give me all persons named "Bill" whose mother is named "Eve"
//
// SELECT persons.*
// FROM persons
// JOIN persons mother ON mother.id = persons.motherId AND mother.name = 'Eve'
// WHERE persons.name = 'Bill'
let request = Person
.filter { $0["name"] == "Bill" }
.join(required: true, mother.filter { $0["name"] == "Eve" })
// With SQLColumn:
let nameColumn = SQLColumn("name")
let request = Person
.filter { $0[nameColumn] == "Bill" }
.join(required: true, mother.filter { $0[nameColumn] == "Eve" }) The The filter closure is new. It allows GRDB to postpone the table aliases generation until the request is translated into SQL. A downside is that it only helps writing conditions on a single table: // Still supported:
Person.filter(nameColumn == "Bill")
// When there are joins:
Person.filter { $0[nameColumn] == "Bill" } Last, two examples of joins which are not supported by the query interface and require SQL snippets (and sometimes explicit table aliasing): // All persons whose mother is named Eve or born in Europe:
Person.
.join(mother, country)
.filter(sql: "mother.name = ? OR country.continent = ?", arguments: ["Eve", "Europe"])
// All persons whose mother or grand-mother is named Eve:
Person.
.join(mother.aliased("mother")
.join(mother.aliased("grandmother")))
.filter(sql: "mother.name = ? OR grandmother.name = ?", arguments: ["Eve", "Eve"]) At least it is possible to write such query - at one point eventually raw SQL becomes the best option anyway. Writing this has shown me that I still have pretty big issues to solve.... Nevertheless, let's at least deal with table creation ;-) |
I forgot about configuration. However, I'm not sure the configuration API is the appropriate way to expose the
As you say, GRDB is for those who prefer to write SQL and work closer with it. In the absence of this table builder interface someone needs to know the column affinities to create a table anyway, and this DSL just wraps in Swift.
I think the options I prefer to keep the DSL as close to the actual SQL code as possible. The idea is that you know SQL, which is why you would use GRDB, but don't want to use strings. Anything other than
My vote goes strongly for affinity. Perhaps using the label
I suggested
If I leave out that param in the DSL I would expect the generated SQL to look like this: CREATE TABLE foo; Anything else would be a major surprise (especially blob, which is used for very specific purposes). For this reason, the In such case, SQLite's default will be used, which is
I'm strongly against defaulting to I am going to sound like a broken record here, but again I advocate deviating as little from SQLite as possible. It might be convenient for some cases, but in most cases it is just different that what someone would expect.
I tried to do this, but remember that you also need to support the optional conflict clause when you specify a primary key. Doing this separate from the primary key param will allow you to specify a primary key conflict clause even if you have not specified a primary key.
Have a look at my examples again. You can conveniently specify boolean SQL.create(name: "foo", ifNotExists: true)
.column(
name: "id",
affinity: .integer,
notNull: true) Perhaps
This makes assumptions about what the primary key is called. Some people would prefer to use
Indeed. I have seen that documentation. And I agree a developer should avoid using it, but that does not mean it should be excluded in the API. In my example I also provided a way to do this without auto-increment: SQL.create(name: "foo", ifNotExists: true)
.column(
name: "id",
primaryKey: SQL.onConflict(.abort),
affinity: .integer,
notNull: true) Note that ordering is not relevant unless you specify auto-increment. I hope you can see that there are some philosophies that I am strongly following in the design:
When I came up with the DSL, I literally sat in front of the SQLite documentation and translated the grammar piece by piece into Swift. So far my proposal is a nearly 100% complete representation of the SQLite I think we are moving a good direction. From what I can tell the biggest area of difference between us right now is nearly always about what to do by default. I am consistently against doing anything different from SQLite's default semantics, I hope my arguments above convince you this is the right approach. |
Some more comments:
On the fluent API vs the param label: I think the later reads more clearly, and now that I have experimented with the nearly the whole grammar I can see that there were only those few cases that were difficult to figure out. I notice you put You mentioned having schema methods like |
This thread is getting long and there are two parallel conversations. I suggest closing this issue and opening separate ones for join and create discussions. |
Hello @cfilipov, we're diverging again!
SQLite column affinities are not well-known. They are designed to remain semi-obscure to regular SQLite users. SQLite column affinities are a tool used by SQLite to improve its compatibility with other database engines, and welcome a very wide gamut of developers by not punishing them if they lack SQLite expertise. A great use case where affinities show they're not the right tool in designing our API is dates. It happens that GRDB stores dates as strings. It could have stored them as timestamps, or even Julian days, natively supported by SQLite. Choosing strings over timestamps and julian days has been a non-trivial debate. Choosing the precise string format has been a non-trivial debate (SQLite supports two). There are enough people and libraries out there that store dates in another way so that you can understand that "storing dates" is more easily said than done. If we had only affinities in our table creation DSL, the user would hesitate as soon as he has to store a date. And users could not easily grasp the "storage intent" when reading the migration code: t.column("date", .string) // meh
t.column("date", .double) // this guy didn't read GRDB documentation
t.column("date", .datetime) // of course, ok let's move on @cfilipov It's high time you to start criticizing your opinions from the point of view of the user.
I know understand that you did not propose this feature to solve a problem or answer a use case. You did propose this feature in your forced march to reimplement SQLite grammar in Swift. My interest instantly dropped to zero (more on that below).
SQL is a much better medium to itself than a dry and verbose Swift API that painfully mimicks its target language. My interest for this scholar exercise is zero. My interest goes to designing an API that takes in account the reality of developers' tasks. BTW I dislike magic just as much as you do. db.schema.create(table: "persons") { t in
t.column("name", .text, default: "Anonymous")
t.column("birthDate", .date)
t.column("email", .text, unique: true, collate: .nocase)
} The code above is not that bad at readability. Better than your current proposal, quite obviously. Defaulting to "id INTEGER PRIMARY KEY" is maybe blunt: I'll look at other table creation DSLs to forge a stronger opinion. db.schema.create(table: "citizenships", primaryKey: ["personId", "countryIsoCode"]) { t in
t.column("personId", .integer, null: false)
.references("persons", onDelete: .cascade, onUpdate: .cascade)
t.column("countryIsoCode", .text, null: false)
.references("countries", onDelete: .cascade, onUpdate: .cascade)
} We're using schema inspection to infer the referenced primary keys above (no defaulting to
Studying the SQLite grammar is great. What you forgot in translating it into Swift is considering the users of your APIs. You had to spoil the mainstream use cases to leave room for less-used grammar corners. Your intellectual skills are already strong, now maybe the time to put them at designing APIs for other developers. |
I suspect we actually agree closer than you think. Part of the creative process is putting out ideas that might end up being dumb, look at them with an open mind and try not to take everything I say as an attack on your opinion. Likewise I will assume the best intentions from you.
I don't understand what you mean by obscure. Every time you create a table you are calling out affinities. Every SQLite tutorial, SQLite's own docs, everything I find mentions these affinities (though they might call them "types"). This is what a table create statement looks like, and I find nothing about it to be obscure: CREATE TABLE person (
id INT PRIMARY KEY NOT NULL,
name TEXT,
); I am proposing that the DSL follow the same options we are familiar with. Once again, we both agree that writing SQL directly is the best and more powerful way to work with SQLite. My goal with the DSL is to maintain this power but without resorting to working inside strings within Swift.
SQLite doesn't natively store dates, you can store dates as strings or an integers and SQLite comes with some built in functions to make them easier to work with. But they are just strings and integers (depending on what you choose). As you mentioned earlier, the storage and retrieval of dates is controlled by the query API. Marking this table as a
This will result in a create statement that generates a
The use case is for a developer that knows SQLite and wants to interface directly with SQL, but doesn't want to do it with strings. This isn't a forced march, it was simple and intuitive to implement, and I find it to be simple to use because of the similarity. I have used some SQL DSLs before and the ones I didn't like tried too hard to be original and fancy and diverged enough from SQL that you needed to re-learn everything in order to use them. I think someone who's used to writing SQL should be able to easily use the DSL after learning the builder syntax, and not have to refer to documentation every time because it diverges from what SQL does for a similarly-named part.
I am surprised that you described GRDB as a tool for those who want to interface directly with SQLite, who prefer to work with SQL, and then claim that staying too close to SQLite's semantics is the wrong approach. My intent is to make it usable and intuitive by following patterns one would expect from a SQL DSL.
I agree that this form is likely the right way to go. What I like about this is the fact that each column is a single statement compared to doing it without the closure (where the whole thing is a single expression and we'd rely on a method to return a column builder to start a new column).
I still think that primary key should be done outside of the db.schema.create(table: "citizenships") { t in
t.column("personId", .integer, null: false)
.references("persons", onDelete: .cascade, onUpdate: .cascade)
t.column("countryIsoCode", .text, null: false)
.references("countries", onDelete: .cascade, onUpdate: .cascade)
t.primaryKey(columns: "personId", "countryIsoCode") // optionally specify conflict clause etc...
}
You like to make many board generalizations in your critiques but I think it would help to be more specific and technical. As I said, I have been consistent in targeting the user who like SQL and want to continue to write SQL but avoid working in strings. To that end I have proposed an API that focuses on them. Please be specific, what mainstream use cases am I spoiling? It's hard to argue general points when you do not call out a specific part of the API. |
SQLite affinities Before SQLite was SQL, and the SQL standard. SQL defines various column types, that standard-compliant SQL implementors have to deal with, in a way or another: CREATE TABLE persons (
id INTEGER, -- 1
name VARCHAR(255), -- 'Arthur'
birthDate DATE -- 1987-09-18
lastUpdate DATETIME, -- 2016-07-05 19:43
picture BLOB -- <data>
) INTEGER, DATETIME and BLOB are SQL types. There are many, and the list is extending. Database engines implement them in various and sometimes inconsistent ways. At the time SQLite was designed, the major databases where Oracle, SQLServer, Sybase, MySQL, PostgreSQL, and I surely overlook some important ones. Dwayne Richard Hipp, the author of SQLite, decided that it was important that his tiny SQL library would be mostly compatible with them. Quoting https://www.sqlite.org/datatype3.html:
And indeed the way SQLite deals with SQL types is funny. It is a mix of Storage classes, C types, and Affinities. There are five storage classes: int64, double, text, blob, and null. These are the actual data stored in the SQLite file. A single column may contain values of multiple storage classes. There are several supported C types: There are five affinities: TEXT, NUMERIC, INTEGER, REAL, BLOB. They are the bridge between C types and storage classes: affinities define which storage class will be used depending on the C type of the value stored by the SQLite user. For example, when you store the C Store the C Store the C The last example is odd, of course - yet it is real, even if I doubt it is much used. I wanted to show you the strange beasts affinities are. Not as concrete as storage classes and C types, affinities are algorithms. For more details, see https://www.sqlite.org/datatype3.html. It's very interesting. Back to our topic. Storage classes, affinities, and C types are how SQLite implement the SQL types defined by the SQL standard. An SQLite expert needs to know about them, of course. Still, the types that are designed to be user-facing are SQL types: SQLite gives us this list:
This list is much too much too long for our purpose, of course. Yet: when the user creates a table, we'll stick to the data types that the standard has designated as the types used to create a table. And those are SQL Types. Not affinities, not storage classes, not C types. Quoting our beloved Swift 3 API Design Guidelines: Don’t surprise an expert, don’t confuse a beginner. Thus the list of types of our DSL could be: End of the course about SQLite affinities ;-)
And now you know that The DATETIME SQL type has the affinity NUMERIC. That affinity is the most funny of all, in that it tries to convert everything to numbers, but preserves values that can't be converted: it will store 30000 for the string "3.0e+5", "foo" for "foo", and... "2016-07-05" for "2016-07-05". That means that it is compatible with the fact that GRDB stores dates as strings. But one could also store timestamps, no problem. Oh, SQLite!
Yes, I wanted to shake you a little :-) But you're right, I'll do what you ask for the best I can. Plus you have made very valid observations. See you there in a couple of hours! |
Thank you for the explanation, this clears things up a lot. I misunderstood the grammar as accepting an affinity instead of a type, when in reality SQLite accepts all the types you would expect in a SQL create statement. I think what makes this even more confusing is that many tutorials and stackoverflow answers encourage the use of only types that map directly to one of the SQLite storage classes (as in, sticking to the types "TEXT", "INTEGER", etc...). I've never seen anyone create a "DATETIME" column in SQLite, even though it's valid. After reviewing the docs you linked I can see why I was wrong about exposing affinity and I now agree that SQL types would be the most appropriate. The question is, what types should be exposed? Everything (for completeness)? If not everything, then what subset would be appropriate?
Very much agreed. Question: Several parts of the CREATE TABLE foo ( id INTEGER PRIMARY KEY CHECK ( id > 10 ) ) This can be represented in the DSL like so: try db.create(table: "foo") { t in
t.column(id, type: .integer)
.check(id > 10)
} Conveniently, I was able to re-use the CREATE TABLE foo ( id INTEGER PRIMARY KEY CHECK ( id > ? ) ) Unfortunately, this can't be done in the create statement because you can't use parameters there:
Of course, the simple workaround is to just use inline SQL: try db.create(table: "foo") { t in
t.column(id, type: .integer)
.check(sql: "id > 10")
} I can see a few ways to make this work, but I suspect there may be problems. What are your thoughts? |
Regarding your joins comment:
I agree about the word variant. I can't think of a much better term. How about I like the way the API looks so far. What about selecting columns? Perhaps I want to have an aggregate column to return the max age of all grandparents, for example. This would of course require it to be done at the row level or perhaps even requiring one to define a |
Hello @cfilipov, Let's compare our two present options, last time we attempted at writing actual code: // fluent/builder/params
db.execute(SQL.create(table: "foo", ifNotExists: true)
.column(
name: "id",
affinity: .integer,
notNull: true,
primaryKey: true),
.column(name: "int", affinity: .integer)
.column(
name: "bar_id",
affinity: .integer,
references: SQL.foreignKey(
table: "bar",
onDelete: .cascade,
onUpdate: .cascade
deferrable: true,
columns: "id"))
}
// closure, one statement by column
db.schema.create(table: "foo", ifNotExists: true) { t in
t.column("name", .integer)
t.column("bar_id", .integer)
.references("bar", onDelete: .cascade, onUpdate: .cascade, deferrable: true)
} I'm happy that you aggree that one statement per column is the right way to go. For me it's just because a huge expression looks impressive, and I don't like to write impressive code unless I really have to :-) You were writing: "You mentioned having schema methods like create only be available during migrations. You have good intentions but [...]". I agree. I'll try below without even OK, let's go: Table creation extension Database {
func create(table tableName: String, temporary: Bool = false, ifNotExists Bool = false, body: (t: SQLTableBuilder) -> ())
}
db.create(table: "foo") { t in ... }
db.create(table: "foo", temporary: true, ifNotExists: true) { t in ... } This gives us:
I was planning to give the primary key columns here, and you answered "but remember that you also need to support the optional conflict clause when you specify a primary key" - OK no primary key columns in this method. Conflict clauses on primary keys are rare, but useful: let's keep them in the scope of our DSL. Columns (basics) struct SQLTableBuilder {
func column(_ name: String, _ type: SQLType, ...)
}
enum SQLType: String {
case text = "TEXT"
case integer = "INTEGER"
case double = "DOUBLE"
case numeric = "NUMERIC"
case boolean = "BOOLEAN"
case blob = "BLOB"
case date = "DATE"
case datetime = "DATETIME"
}
db.create(table: "person") { t in
t.column("name", .text)
t.column("age", .integer)
} Not naming the parameters of
Columns (nullability) Let's look at various options:
I'm not convinced by the options set at all, and the fluent way has more punctuation: I'll try with a parameter. Yet we'll maybe have to look back at this choice later. The SQLite grammar accepts a conflict clause on a struct SQLTableBuilder {
func column(
_ name: String,
_ type: SQLType,
null: Bool = true,
onNull: SQLConflict = .abort
...)
}
enum SQLConflict : String {
case rollback = "ROLLBACK"
case abort = "ABORT"
case fail = "FAIL"
case ignore = "IGNORE"
case replace = "REPLACE"
}
db.create(table: "person") { t in
t.column("name", .text, null: false)
t.column("age", .integer, null: false, onNull: .rollback)
} The onNull conflict is ignored unless the null parameter is false. Since abort is the default conflict resolution algorithm, we can avoid generating a confict clause for this specific value:
I give another option below, but I like your t.column("name", .text, null: .allow)
t.column("name", .text, null: .disallow)
t.column("name", .text, null: .disallowWithConflict(.rollback)) Columns (unique) Just as struct SQLTableBuilder {
func column(
_ name: String,
_ type: SQLType,
null: Bool = true,
onNull: SQLConflict = .abort
unique: Bool = false,
onDuplicate: SQLConflict = .abort
...)
}
enum SQLConflict : String {
case rollback = "ROLLBACK"
case abort = "ABORT"
case fail = "FAIL"
case ignore = "IGNORE"
case replace = "REPLACE"
}
db.create(table: "person") { t in
t.column("name", .text, unique: true)
t.column("age", .integer, unique: true, onNull: .rollback)
}
Your initial idea about unicity was fluent: Should we eventually pick a fluent interface, the nullability should be updated accordingly: db.create(table: "person") { t in
t.column("name", .text).notNull().unique() // default conflict: abort
t.column("age", .integer.notNull(onConflict: .rollback).unique(onConflict: .rollback)
} Why not, eventually? Let's keep on. Columns (default value) struct SQLTableBuilder {
func column(
_ name: String,
_ type: SQLType,
null: Bool = true,
onNull: SQLConflict = .abort
unique: Bool = false,
onDuplicate: SQLConflict = .abort,
default defaultValue: DatabaseValueConvertible,
...)
}
db.create(table: "person") { t in
t.column("name", .text, default: "Anonymous")
}
Easier said than done, though: just like the CHECK constraints you have already studied, The actual SQLite grammar accepts CURRENT_TIME, CURRENT_DATE and CURRENT_TIMESTAMP as default values. OK, seriously, do we really have to support them, when inserting a date created in Swift has the same effect? Not supporting them lets us use the simple DatabaseValueConvertible protocol, and throw any value in. Should we support the special literals, we'd have to write
OK, a little pause. On the remaining list: Columns:
Table constraints:
Other:
Table alterations:
Open questions:
|
That's an excellent suggestion!!! I should become a native English speaker some day. If I try to rewrite the row adaper documentation, we get: let authorMapping = ColumnMapping(["id": "authorID", "name": "authorName"])
let adapter = ScopedAdapter(scopes: ["author": authorMapping])
for row in Row.fetch(db, sql, adapter: adapter) {
if let authorRow = row.scoped(on: "author") { ... }
} Pretty cool! A big thank you! And it fits well the join context as well :-)
I don't have any answer yet. About aggregates, I have bookmarked this link as food for thought. It reads:
This is my only contact so far with a DSL which deals with such a use case. And selecting columns, even without aggregates, is still on the TODO list as well. It's still a long way! |
Yes, maybe it is the right time for that, now. |
BTW - I'm having one week of vacations starting tomorrow - I won't be there much. Have fun on our migration DSL meanwhile :-) |
Enjoy your vacation! I've been working on responses to both the join and create topics and will continue those discussions as new, separate issues. So I will close this one and refer to it in the new issues. I should have some replies waiting for you when you get back. Enjoy. |
Apple itself is using keywords such as 'in' in argument labels. For example, check "Refined API Naming" section on their website: |
The |
WOW - that was a long thread :) |
Are there any plans for supporting SQL
join
in the query interface? Along the same lines.. what about creating tables?Edit: Just noticed the TODO.md mentions joins and a few other nice things. But not table creation.
The text was updated successfully, but these errors were encountered: