-
-
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
Enhancement: Allow initializing rows using a mapping #50
Comments
@schveiguy Thanks for pushing the topic of associations. When I read your question, it looks that the SQL query is built by some object (that fetches some items), and the adapter built by some other object (likely in the Item1 initializer). If so, the design looks brittle. Wouldn't it be better if the same object was building both the SQL query and the adapter?
Yes, that's definitely a better fit. Assuming I understand what you want, my first attempt at a solution would be the following: I introduce an Adapter type that has a main mapping, and mappings for "sub rows". Rows fetched with an adapter are seen through the main mapping. And rows can be asked for the eventual presence of sub rows: let sql = "SELECT item1.id AS item2_id, item1.val AS item1_val, " +
"item2.id AS item2_id, item2.val AS item2_val " +
"FROM item1 LEFT JOIN item2 ON (item2.id = item1.item2_id)"
// Main mapping targets Item1. A sub row named "item2" targets Item2:
let adapter = Adapter(
mapping: ["id" : "item1_id", "val" : "item1_val"],
subRows: ["item2": ["id" : "item2_id", "val" : "item2_val"]])
// Fetch rows:
for row in Row.fetch(db, sql, adapter: adapter) {
// The main mapping is active:
row.value(named: "id") // column "item1_id", actually
// Row can be asked for the eventual presence of a "sub row":
if let row2 = row.subrows["item2"] {
row2.value(named: "id") // column "item2_id", actually
}
}
// Fetch item:
let items = Item1.fetchAll(db, sql, adapter: adapter) RowConvertible types can still use their favorite column names in their initializers, and use sub rows if needed: struct Item1: RowConvertible {
let id: Int64
let val: String
let item2: Item2
init(_ row: Row) {
// Works both with and without adapter:
id = row.value(named: "id")
val = row.value(named: "val")
guard let row2 = row.subrows["item2"] else {
// Here Item1 really wants its item2.
// But another logic could be implemented.
fatalError("Missing item2")
}
item2 = Item2(row2)
}
}
struct Item2: RowConvertible {
let id: Int64
let val: String
init(_ row: Row) {
// Works both with and without adapter:
id = row.value(named: "id")
val = row.value(named: "val")
}
} Would such a pattern fit your needs? |
Tests for a first draft: https://github.com/groue/GRDB.swift/blob/04e648b3f900e2a385d1cbd318ce257b82cae15c/Tests/Public/Core/Row/RowAdapterTests.swift I still have questions about mapped row equality (row equality is important, especially for FetchedRecordsController), and what to do with mappings which refer to non-existing columns (should we crash, or not). |
I'm new to your library, and not an expert in this domain, so I'm going to defer to you in terms of what design works best. My real point is simply that I want to minimize the calls to the DB, and the initializer that accepts a Row is currently rigid in terms of what fields map to members. In my concept, since RowConvertible (and not Persistent) needs something external to design/build the query and generate the rows, I did it how I would design a query to get such data :) I actually wasn't even thinking about initializing the Item2 inside the Item1 initializer, just the immediate need to be able to take some Row data, however it was created, and pass to Item2 initializer. But you are right that it's advantageous to be able to initialize Item1's Item2 instance inside Item1's initializer.
Definitely! I'm actually thinking about your answer to my previous question about an aggregate query (for an array), and see that subrows might be useful there as well.
I have no experience with, and probably won't use for my application, the FetchedRecordsController, so I'm unsure how to respond to that.
So you mean, you pass subrow to Item2, but subrow defines a mapping to a column not included in the actual row? Given that the subrow adapter and the row itself likely come from the same place, wouldn't this be a programming error? |
I see what you mean in the first draft test about the missing id. I think this should crash, IMO, and probably upon the fetch. Edit: reasoning is because missing id being nil may be an expected condition for instance if you have a subitem that sometimes is not present. How to distinguish this from poorly constructed query/adapter? |
Tell me more about your initial vision, if you don't mind. |
I can, but not today. I'm about to go off for the weekend, we can perhaps pick this up on Monday. |
So my initial vision for this enhancement was based on the model I have. There is one table that represents entities in a tree. Each entity can have zero or more attributes assigned to them. Each attribute is stored in another table with customized fields that define the attribute. Each entity can only have one attribute from a table, so the primary key is the same number as the entity primary key. So I can fetch the entire entity description with one fetch using joins. If there isn't a matching attribute in that table, then its fields are nil. However, it's possible that each attribute has fields named similarly to attributes in other tables. So in order to fetch them all, I may have to rename them. But then, if I mark my actual attribute struct as RowConvertible, I have to either always fetch them with the renamed fields, rename the fields to something unique, or avoid the initializer that takes a row. I wanted some way to utilize the row initializer, but to use the renamed fields needed to distinguish from other fields in the join. |
@schveiguy I see a mismatch between our views. My proposal won't expose the raw row (with the original column names) when it looks that yours does rely on them. Actually, when I read your proposed code: var adapter = ["id" : "item2_id", "val" : "item2_val"]
let item2 = Item2(row, withAdapter: adapter) // proposed usage? ... I wonder why you did not use a plain initializer: struct Item2 {
init(id: Int, val: String) { ... }
}
let item2 = Item2(id: row.value(named: "id"), val: row.value(named: "val")) |
No, I don't care to get the orginal row data fields. The less the initializer knows the better. The proposal was my poor attempt to add the ability to map the fields to the initializer. Your way is superior.
Because the example is obviously just simple :) The true initializer may be more complex, and I don't want to have to construct some initializer tree so I can avoid code duplication. I just wanted to reuse the initializer I already have. |
OK 😄 I think that the support for row adapters is solid now. I just have to write some sensible documentation and bikeshed method names. Next time you'll need such a pattern, it will likely be ready in GRDB! |
thanks! |
@schveiguy Here is the documentation: https://github.com/groue/GRDB.swift/tree/RowAdapter#row-adapters What do you think? Shippable? |
Looked over the docs. Seems good! |
OK then: GRDB v0.67.0 is out, with support for row adapters. Thanks for your suggestion, it will help dealing with joined queries a lot! |
Thanks for your request @schveiguy. Just used row adapters today, with much profit :-) |
@schveiguy Sorry to disturb you again. I'm happy with the feature, but not with the way things are named, after a little survey with my coworkers. I plan to ship a refactored version that, I hope, will be more clear. The revised version is at https://github.com/groue/GRDB.swift/tree/RowAdapter#row-adapters. Please share your comments if you have any. |
As I haven't yet had an opportunity to use it, the naming hasn't yet crossed my mind :) I can see the problem with the name |
@schveiguy GRDB v0.72.0 has refactored row adapters. Check out the new documentation. |
@schveiguy GRDB v0.75.0 has refactored (again) row adapters. What was called "variant" is now called "scope", and I have good hope the API is stable now. See the new doc and the API diff. cc @cfilipov who suggested this excellent renaming |
RowConvertible takes a Row object, and allows one to initialize an object based on the fields in the row.
However, you may fetch more than one object per row with a join. If you wanted to intialize two objects, you may rename some of the fields to avoid collisions. But the initializer is expecting certain names in the row to match what it is looking for.
A basic example is let's say you have 2 types of items. Item1 has at most one Item2. Each of these match objects that can initialize from a row. However, each one expects the field "id" to map to it's id. You can fetch all rows together from the DB, as one Row object, using a join. For example:
But now, I cannot use the nice init function which takes a row to construct them, I have to use normal construction fetching the correctly named fields.
An adapter may be useful here, for example:
Alternatively, you could make a Row constructor that uses another Row as backing with the adapter on top.
The text was updated successfully, but these errors were encountered: