yahoo / squidb

SquiDB is a SQLite database library for Android and iOS
https://github.com/yahoo/squidb/wiki
Apache License 2.0
1.31k stars 132 forks source link

User set primary key values #226

Closed Aranda closed 8 years ago

Aranda commented 8 years ago

This is more of a question as I may be missing something, or using SquiDB in a way it was not designed for.

Is there a way for me to manually set the value of my @PrimaryKey column such that persist will still write it? The reason being that I am retrieving primary key values from the server, so it makes sense for me to specify them before persisting with SquiDB.

The workaround I have is to use a UNIQUE constraint on my primary key column with nowRowIdAlias = true, so SquiDB creates it's own rowid, then also use persistWithOnConflict(..., REPLACE).

This doesn't seem ideal though as I now essentially have two primary keys and SquiDB's one gets changed every time I want to Update.

sbosley commented 8 years ago

You're definitely on the right track. The way persist() works is that it attempts to perform an insert operation if the rowid is not set, and an update operation when the rowid is not set -- the rowid is how SquiDB tries to do bookkeeping about whether or not the model "exists" yet. This is true regardless of whether the model uses the internal SQLite rowid column or an integer primary key alias (note that any integer primary key is always an alias for the rowid; that's just what SQLite does).

If you want to change the semantics of persist() the best way to do so is to define a custom method in your SquidDatabase subclass that delegates to the methods insertRow() and/or updateRow(). These methods perform insert/update operations respectively using whatever values are in the model, rowids or primary keys included. (And in fact, you'll see that our default persist() is implemented using them under the hood, so you can look there to get a sense of how they can/should be used).

Also worth noting: if you use noRowIdAlias = true and specify a custom primary key using @PrimaryKey, that primary key shouldn't need any additional UNIQUE constraints -- SQLite implements PRIMARY KEYs using unique constraints under the hood (see this doc for details).

Hope that helps!

sbosley commented 8 years ago

Also just FYI if you have questions in the future and if you're not sure whether or not to open an issue, you can join our new Gitter chat at https://gitter.im/yahoo/squidb and someone there will try to help you out.

Aranda commented 8 years ago

Great, that covers all my queries. I guess the way forward is to do as you suggest and make a custom persist method, like insertOrUpdate. I'd have to then query the db to see if a row with the given rowid exists to decide whether to use insertRow or updateRow. Still on the right track? :)

sbosley commented 8 years ago

Yep still seems reasonable to me. I might not query the db for row based on the rowid though, but rather on your primary key value (since that's sort of the "identifier" for the model in your case it sounds like). fetchByCriterion() is a good method to use for that kind of thing.

And I've done it both ways actually -- you can either query to see if the row exists, merge the model values if so, and then persist again, or you can do an insert with a REPLACE conflict resolution like you suggested earlier. Both are good approaches, just with slightly different SQL semantics -- you can decide which works best for your use case.

Aranda commented 8 years ago

Ok, we'll I'm hoping to just use @PrimaryKey on my DB key columns, which as I understand maps the rowid to that column (at least in the case of integer primary keys). So this is what I have, which seems to be working so far:

public boolean insertOrUpdate(TableModel item) {
    Class<? extends TableModel> modelClass = item.getClass();
    TableModel existingItem = fetch(modelClass, item.getRowId());
    if (existingItem == null) {
        return insertRow(item);
    } else {
        return updateRow(item);
    }
}

Thanks for the help!

sbosley commented 8 years ago

Yep that definitely works. Two very minor suggestions:

updateRow() will only attempt to update the values found in the setValues of the item -- and items are only added to setValues if something has changed. You could do something like this:

public boolean insertOrUpdate(TableModel item) {
    Class<? extends TableModel> modelClass = item.getClass();
    Table table = getTable(modelClass);
    TableModel existingItem = fetch(modelClass, item.getRowId());
    if (existingItem == null) {
        return insertRow(item);
    } else {
        // For any set value in item for the given set of properties, update the existing model with those values
        existing.setPropertiesFromValuesStorage(item.getSetValues(), t.getProperties()); 

        // Update the existing model with the new set values. This is a no-op if nothing has changed
        return updateRow(existing);
    }
}

Comparing/merging the values in memory can save you a round trip to the DB if nothing has changed -- which may be the case if you're syncing from server. Your version is totally fine as is, so don't feel like you have to change anything if it's working well for you -- just a minor suggestion that you can experiment with if you feel so inclined.

The other suggestion only applies if you're accessing the DB multi-threadedly -- wrapping the read/write pair in a transaction (or some other form of synchronization) can prevent two parallel threads from trying to insert the same model at the same time.

Aranda commented 8 years ago

Wonderful!

My only thought now is how to handle non-integer @PrimaryKey fields generically. As I understand, using @PrimaryKey String someField; will require me to have a specific insertOrUpdate function that uses a query with my unique String key since item.getRowId() will be NO_ID for items just generated from server returns.

Is there some way to generically get the @PrimaryKey value of the TableModel, even when it doesn't map to the rowid?

sbosley commented 8 years ago

Great question. In the past, I've done something like this:

// This accepts multiple logical keys, which is nice for things that may have a multi-column
// primary key, or are just unique on some combo of fields
public <T extends TableModel> boolean insertOrUpdate(T item, Property<?>... logicalKey) {
    // Validate the arguments
    if (logicalKey == null || logicalKey.length == 0) {
        throw new IllegalArgumentException("No logical key specified");
    }
    for (Property<?> col : logicalKey) {
        if (!item.containsNonNullValue(col)) {
            throw new IllegalArgumentException("Item did not have a value to compare for logical key column " + col);
        }
    }

    // Build a criterion for checking for existing models with the given logical key
    Criterion existingCriterion = logicalKey[0].eq(item.get(logicalKey[0]);
    for (int i = 1; i < logicalKey.length; i++ ) {
        existingCriterion = existingCriterion.and(logicalKey[i].eq(item.get(logicalKey[i])));
    }

    boolean result;
    Table t = getTable(item.getClass());
    T existing = (T) fetchByCriterion(item.getClass(), existingCriterion);
    if (existing == null) {
        result = insertRow(item);
    } else {
        existing.setPropertiesFromValuesStorage(item.getSetValues(), t.getProperties());
        result = updateRow(existing);
        if (result) { // This is not necessarily required, but it mimics the bookkeeping that persist() does
            item.setRowId(existing.getRowId());
            item.markSaved();
        }
    }
    return result;
}

Again, you can decide if you want to wrap this in a transaction or use some other synchronization mechanism.

This is a little bit more work than extracting an arbitrary primary key from the TableModel -- you have to know which column(s) to use for the logical key when calling the method. We haven't had this use case enough times for this to make it into core, but maybe you could also write a code generation plugin to make table models expose their primary keys somehow? Could be an interesting enhancement, I'll think more about if it makes sense for SquiDB itself, but you can also experiment with the plugin API on your own to see if it's helpful. The wiki page on plugins can be found here, and I recommend taking a look at how TableModelSpecFieldPlugin handles primary keys if you want to go this route.

Aranda commented 8 years ago

Again, thank you!. Writing a plugin is a little more in-depth than I want to get for this project, but your solution with logical keys is perfect. I'll use gitter chat for further discussion if necessary.

As for whether it makes sense for TableModel to know about it's primary key, I'll leave that up to your judgement, but it seems that matching external data (and hence specifying the PK before inserting) would be a reasonably common use case.