Sqldelight: Allow computed fields based on database fields (HIDE in compiler)

[droidpl]

A way I have found that works pretty well with sqldelight is to have two different models separated, one for the database model and another for the data that comes from network. Thus sqldelight forces you to some types based on the database logic (that can be different to the presentation logic) with autovalue it gets a bit messy to mix Database, network and presentation in the same model.

Said that, now when we have a factory, this factory needs a generic class that extends your SomethingModel, but what if I want to have a FACTORY object that provides me the mapping between the presentation and the database without creating a new instance of the database object, that will avoid huge memory allocations with big lists.

Let me show an example. I have two classes: Comment and CommentDb. Comment is my presentation which can have an id null and CommentDb is my database layer.

Comment.java

@AutoValue
public abstract class Comment {

    @Nullable
    public abstract Long id();

    @NonNull
    public abstract Long postId();

    @NonNull
    public abstract String name();

    @NonNull
    public abstract String email();

    @NonNull
    public abstract String body();

    @NonNull
    public abstract Long createdAt();

    @Nullable
    public abstract Long updatedAt();

    @Nullable
    public abstract Long deletedAt();

    public abstract boolean needsSync();

    @NonNull
    public ContentValues marshal(){
        return CommentDb.FACTORY.marshal()
                ._id(id())
                ._postId(postId())
                ._name(name())
                ._body(body())
                ._email(email())
                ._createdAt(createdAt())
                ._updatedAt(updatedAt())
                ._deletedAt(deletedAt())
                ._needsSync(needsSync())
                .asContentValues();
    }

    @NonNull
    public static Builder builder() {
        return new AutoValue_Comment.Builder();
    }

    @NonNull
    public static Builder builder(@NonNull Comment comment) {
        return builder()
                .postId(comment.postId())
                .id(comment.id())
                .name(comment.name())
                .email(comment.email())
                .body(comment.body())
                .createdAt(comment.createdAt())
                .updatedAt(comment.updatedAt())
                .deletedAt(comment.deletedAt())
                .needsSync(comment.needsSync());
    }

    @AutoValue.Builder
    public static abstract class Builder {

        @NonNull
        public abstract Builder id(@Nullable Long id);

        @NonNull
        public abstract Builder postId(@NonNull Long postId);

        @NonNull
        public abstract Builder name(@NonNull String name);

        @NonNull
        public abstract Builder email(@NonNull String email);

        @NonNull
        public abstract Builder body(@NonNull String body);

        @NonNull
        public abstract Builder createdAt(@NonNull Long createdAt);

        @NonNull
        public abstract Builder updatedAt(@Nullable Long updatedAt);

        @NonNull
        public abstract Builder deletedAt(@Nullable Long deletedAt);

        @NonNull
        public abstract Builder needsSync(@Nullable boolean needsSync);

        @NonNull
        public abstract Comment build();
    }
}

CommentDb.java

@AutoValue
public abstract class CommentDb implements CommentModel {

    public static final Factory<CommentDb> FACTORY = new Factory<>(new CommentModel.Creator<CommentDb>() {
        @Override
        public CommentDb create(long id, long postId, String name, String body, String email, long createdAt, Long updatedAt, Long deletedAt, boolean needsSync) {
            return new AutoValue_CommentDb(id, postId, name, body, email, createdAt, updatedAt, deletedAt, needsSync);
        }
    });

    public static final RowMapper<CommentDb> COMMENTS_POST_MAPPER = FACTORY.selectCommentsPostMapper();

    public static final RowMapper<CommentDb> SYNC_POSTS_MAPPER = FACTORY.selectSyncCommentsMapper();
}

Said that, I would like to be able to do something like:

public static final Factory<Comment> FACTORY = new Factory<>(new CommentModel.Creator<Comment>() {
        @Override
        public Comment create(long id, long postId, String name, String body, String email, long createdAt, Long updatedAt, Long deletedAt, boolean needsSync) {
            //build here my comment based on the database info
        }
    });

[AlecStrong]

You might be able to get your desired behaviour with views. Suppose in your presentation layer you only want the postId, name, email and body, you could make a view like

in Comment.sq

comment_presentation:
CREATE VIEW comment_presentation AS
SELECT postId, name, email, body
FROM comment;

select_all_comments:
SELECT *
FROM comment_presentation;

now whenever you write selects that are meant to return the presentation implementation you query your view instead of your table. The factory still takes a Creator<TableModel> but you can pass null for that if you never actually create an instance of your table. So your implementation class can look like:

@AutoValue
public abstract class Comment implements CommentModel.Comment_presentationModel {
  public static final Factory<CommentModel> FACTORY = new Factory<>(null);

  public static final RowMapper<Comment> ALL_COMMENTS = FACTORY.select_all_commentsMapper(new CommentModel.Select_all_commentsCreator<Comment>() {
    @Override
    public Comment create(long postId, String name, String email, String body) {
      return new AutoValue_Comment(postId, name, email, body);
    }
  });
}

passing null to the factory isnt great and will be solved with #358.

Let me know if this helps out or there is anything i missed while reading your issue

[droidpl]

Yeah passing null to the factory doesn't look good at all. I still see a small concern since I am exposing directly, because of the interface, the methods of my database view (and because of autovalue creating some fields on this object) but maybe I want in my presentation something processed.

Lets say I have a discount percentage and a price in my database, the view brings both and in my Presentation object I am showing only the final price. In that case I want to make that process in the factory or in the constructor, but avoid having both fields there. It is just a minor issue but still would make SqlDelight way more awesome.

Let me know what you think.

[AlecStrong]

yea i've been thinking about that issue a lot recently as well. The problem I've had is that there are times you want your view (presentation) to perform some computation on the model and expose that instead of the columns it is performing the computation on. SQLite has a limited vocabulary of functions to do this with so it's not often possible to do it exclusively through SQLite.

Sample of problem:

User.sq :

CREATE TABLE user (
  _id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
  name TEXT NOT NULL,
  email TEXT,
  sms TEXT
);

name_addresses:
SELECT name, group_concat(email), group_concat(sms)
FROM user
GROUP BY (name);

if I want to have a single member on the view addresses with type List<String> then I end up with an implementation that looks like this:

@AutoValue
public abstract class NameAddresses implements UserModel.Name_addressesModel {
  public abstract List<String> addresses();

  @Override
  public String group_concat_email() {
    throw new UnsupportedOperationException("Call addresses() instead");
  }

  @Override
  public String group_concat_sms() {
    throw new UnsupportedOperationException("Call addresses() instead");
  }
}

the type AutoValue_NameAddresses will have constructor AutoValue_NameAddresses(String name, List<String> addresses) so it isn't wasting space storing columns used in computation.

The implementation of the table looks like

@AutoValue
public abstract class User implements UserModel {
  public static final Factory<User> FACTORY = new Factory(AutoValue_User::new);

  public static final RowMapper<NameAddresses> NAME_ADDRESSES_MAPPER = FACTORY
    .name_addressesMapper((name, group_concat_email, group_concat_sms) -> {
      List<String> addresses = new ArrayList<String>();
      addresses.addAll(Arrays.asList(group_concat_email.split(",")));
      addresses.addAll(Arrays.asList(group_concat_sms.split(",")));
      return new AutoValue_NameAddresses(name, addresses);
    });
}

The clear drawback is that the type NameAddresses still has functions for group_concat_email() and group_concat_sms() that throw runtime exceptions.

With that in mind it might be sufficient just to hide those functions in the generated interface through a SQLDelight-specific keyword. Example:

name_addresses:
SELECT name, HIDE group_concat(email), HIDE group_concat(sms)
FROM user
GROUP BY name;

then this would generate the interface

interface Name_addressesModel {
  String name();
}

but the Creator would still have the same signature that includes String group_concat_email and String group_concat_sms so you can use them in computation. Then my implementation class becomes

@AutoValue
public abstract class NameAddresses implements UserModel.Name_addressesModel {
  public abstract List<String> addresses();
}

how does this sound @droidpl @JakeWharton

[droidpl]

Mmmm the drawback with that is adding the HIDE syntax which is not so Sqlite specific, and a bit hidden as well to add it to the documentation. To me all of this sounds like working around the current implementation. What about removing the constraint in you Creator so it can be a creator of everything? Or adding a second type, one for the Model class and another for the Mapped class.

[AlecStrong]

We can't relax the constraint in the Creator because Mapper's rely on it to provide typesafe models back to consumers.

If all you need is a second type you could create a second factory with a different creator for your second type

[droidpl]

Maybe not relaxing, but extending it to:

new ProductDb.Creator<Product, ProductDb>() {
        @Override
        public Product create(long id, int price, float percentage) {
            return new AutoValue_Product(id, postId*percentage);
        }
}

Also as mapper relies on it, it should have both types. Again it is not ideal since it makes it a less pretty API. I will keep thinking a bit about it

[AlecStrong]

both types would have to implement the ProductModel interface so you can do that currently with

new ProductDb.Creator<ProductModel>() {
        @Override
        public Product create(long id, int price, float percentage) {
            return new AutoValue_Product(id, postId*percentage);
        }
}

ProductModel is a relaxed supertype that all your subclasses have to implement.

[droidpl]

Yeah like that works but you still have those extra methods in your model as you told before. I was thinking of a solution that avoids those extra methods there.

[AlecStrong]

which is what having a HIDE keyword would solve.

Regardless of solution a Creator will always be parameterized with a type that extends the Model. That will never change in SQLDelight. Without something like a HIDE keyword subclasses will have to implement that method. Using a view you can select only the methods you care about, but if you need a column for computation in the java model then a HIDE keyword could remove the method on the generated interface and avoid that problem.

[droidpl]

Ok, then just a good documentation for it will be needed and a detailed explanation for what you can use it. 👍 Go for the HIDE

[AlecStrong]

@droidpl I've been thinking a lot about this recently and I agree with your original idea. Here's the plan:

Creators will have no type restriction for what they return. All a Mapper will do is give a type-safe representation of a row, you can do whatever you want with it.

We'll still generate the interfaces as a convenience for things like AutoValue. This also means there will be no regression, you can continue implementing the interface and having Mappers return only implementations of the interface.

Sorry it took so long :D This will be a very major change in the SQLDelight API since we're effectively moving from "generating a java object for a row" to "give you a typesafe representation of a row"

[droidpl]

hey @AlecStrong! I had to re-read the whole issue since I completely forgot what was it about :D Yeah I can understand the timings, it is indeed a major change. Sounds good to me, this way the mapping will be way more flexible and also computing fields can be done in an easier way than including more syntax to the database creation.

Thanks for the good work

[AlecStrong]

working-kotlin branch tracking this