Skip to content
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

Always generate query functions with mapper argument #2430

Closed
bubenheimer opened this issue Jun 9, 2021 · 6 comments
Closed

Always generate query functions with mapper argument #2430

bubenheimer opened this issue Jun 9, 2021 · 6 comments
Labels

Comments

@bubenheimer
Copy link

bubenheimer commented Jun 9, 2021

SqlDelight 1.5.0 sometimes does not generate query functions with mapper arguments when the result from a query is a single column. It looks like this happens when the result is a primitive type, perhaps to avoid boxing. However, this prevents the result type from being easily remapped. The current approach also introduces inconsistency: it is difficult to understand the obscure inner workings of the codegen, making results hard to predict for me as a user. Instead of writing simple SQL, I have to hack my SQL to find a tweak to manipulate the codegen into generating a mapper.

The alternative of manually mapping a Query object to a different type of Query object is kind of a mess.

I have a DAO wrapper layer to apply query customizations; this layer operates at the Query object level, not at the Query results level, so I cannot easily re-map query execution results here without the standard provided mapper function.

For example:

CREATE TABLE sample (msgId INTEGER AS MsgId NOT NULL)

max: SELECT ifnull(max(msgId), 0) FROM sample -- result type is Long, 1 query function generated, no mapper
maxTweaked: SELECT max(msgId) FROM sample -- result type is Long?, 2 query functions generated, one with mapper

MsgId is the actual domain type I want in the result; it reserves 0 as a special value.

My actual use case uses a max of a union of max functions for different tables. Some of the tables can have 0 as a proper column value; ifnull combines this case with the case of an empty table.

@bubenheimer
Copy link
Author

This issue is probably more of a bug (e.g. premature optimization) than a feature.

@bubenheimer
Copy link
Author

This is also essential to transform last_insert_rowid() to the appropriate Kotlin type.

@AlecKazakova
Copy link
Collaborator

we do this to avoid unnecessary boxing, but we're also just returning the type that the column supports. Mapping in a lambda vs mapping after executing would have the same performance benefit, so I'm unlikely to make a change here

@bubenheimer
Copy link
Author

I don't see the benefit of not generating the Query class, but I may be missing something. Can't the codegen inline the query to get the no-boxing optimization and still generate the Query class as well, without much extra effort? It's giving up both API orthogonality and the powerful ability to perform transformations purely at the Query level without actively running queries, but for what? A little less codegen and a little less potentially dead code?

@AlecKazakova
Copy link
Collaborator

it still exposes a Query<T>, I'm not sure what API orthogonality you're talking about. It's just that the T isn't boxed

@bubenheimer
Copy link
Author

bubenheimer commented Apr 8, 2022

Sorry, I meant not generating the query function with the mapper argument, it's been a while since I created this issue. So for example, I cannot use mapToList, mapToOne, etc., on a Query with an already transformed type (e.g. MsgId instead of Long), I have to apply the transformation to the flows. With non-primitive types there is no issue with transforming the Query type prior to applying mapToList/mapToOne, so the API usage ends up being very different in these two cases.

SqlDelight encourages the use of custom types, but some queries cannot be readily expressed to return the desired custom type, like last_insert_rowid() or the union/max query I mentioned. So I end up dealing with undesired types at the Query level in some cases, which I can easily mitigate with a mapper function, but in the case of a single primitive return value it's all different, and I cannot do this mitigation, because the query function with a mapper argument is not generated.

I hope this makes sense, I've been writing it from my past recollection. Essentially I think the idea is that with a mapper function I can write my own Dao function to get a Query<B> when SqlDelight gives me a Query<A>, and this is not easily possible without the mapper function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants