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

Support custom mappers for projections with more than 22 columns #1199

Closed
julienbanse opened this issue Jan 31, 2019 · 23 comments · Fixed by #1621
Closed

Support custom mappers for projections with more than 22 columns #1199

julienbanse opened this issue Jan 31, 2019 · 23 comments · Fixed by #1621

Comments

@julienbanse
Copy link

Is it a normal behavior that for queries with a LEFT OUTER JOIN, generated code does not give access to the custom mapper ?

Example :

select_news_with_picture_from_id:
SELECT *
FROM news
LEFT OUTER JOIN author ON news.authorId = author.id
WHERE
news.apiId = ?;

Generated code:

database.newsQueries.select_news_with_picture_from_id(newsId)
               .asObservable()
               .mapToOneNonNull()
@AlecKazakova
Copy link
Collaborator

are there more than 22 columns in the projection?

@julienbanse
Copy link
Author

yes !

@AlecKazakova
Copy link
Collaborator

ha thats fun. There used to be a constraint on 22 parameters to a lambda but thats gone now.

@AlecKazakova AlecKazakova changed the title Access to custom mapper for queries with LEFT OUTER JOIN Support custom mappers for projections with more than 22 columns Jan 31, 2019
@ursusursus
Copy link

I need this as well, I have "polymorphic" items, where all the fields of all subtypes are columns merged in one table

Any workaround?

@AlecKazakova
Copy link
Collaborator

not really, you have to use exposed method and map from the type given to you. I'll try and get a fix in for this before the next release.

@ursusursus
Copy link

Okay, that can work in meantime, thanks

@AlecKazakova
Copy link
Collaborator

@ReginFell
Copy link

Faced with the same issue on 1.1.3 (Android Driver), Kotlin 1.3.30
Any workaround for this?

@AlecKazakova
Copy link
Collaborator

no, we need the above issue fixed first

@ReginFell
Copy link

So is it related to Kotlin Native issue?

We are moving from SqlDelight 0.7.1 and SqlDelight 1.1.3 doesn't serve well in case if we have at least one JOIN (around 25 fields). That can't be simplified, unfortunately.

Do you know if possible to turn off needsLambda check only for Android driver inside sqldelight-compiler and publish it locally? I am stuck with the issue and can't figure out how to resolve it

> Could not create task ':messages:generateDebugDatabaseInterface'.
   > com/intellij/openapi/fileTypes/LanguageFileType
> Could not create task ':smskit:generateDebugDatabaseInterface'.
   > com/squareup/sqldelight/core/lang/SqlDelightFileType

@ursusursus
Copy link

ursusursus commented Apr 26, 2019

Btw the workaround is to map the FooBar projection object to your DomainFooBar manually (so yea, garbage, but will do until fixed)

@ReginFell
Copy link

ReginFell commented Apr 26, 2019

Yeah, this what came to my mind as well, the main issue that I have to do it for every generated interface, even if they have the same fields set.

Also another solution is to map SqlCursor manually, but obviously we can't use autogenerated mappers from SqlDelight and it doesn't make much sense to use Delight without it's main features.

@AlecKazakova
Copy link
Collaborator

if you are exposing the same fields from multiple queries you can use a view to have the interface be the same

@ursusursus
Copy link

ursusursus commented Apr 30, 2019

@AlecStrong view require migrations though 😣

@benasher44
Copy link
Contributor

Hype: JetBrains/kotlin-native#3253

@JakeWharton
Copy link
Collaborator

"Abitrary Arity" is the name of my punk rock band

@aballano
Copy link

Hype: JetBrains/kotlin-native#3253

It's now merged, I asked if it's possible to re-open the original PR

@benasher44
Copy link
Contributor

It's fixed in Kotlin 1.3.60, which is not released

@adenisyuk
Copy link

Any updates on this?

@benasher44
Copy link
Contributor

This is fixed in Kotlin 1.3.60. This can prob be closed.

@adenisyuk
Copy link

adenisyuk commented Feb 17, 2020

@benasher44 ,
But it's not fixed in SqlDelight (probably, this is the PR which can bring a fix: #1232)

Here is a Test.sq:

CREATE TABLE dbTest(
    field1 TEXT,
    field2 TEXT,
    field3 TEXT,
    field4 TEXT,
    field5 TEXT,
    field6 TEXT,
    field7 TEXT,
    field8 TEXT,
    field9 TEXT,
    field10 TEXT,

    field11 TEXT,
    field12 TEXT,
    field13 TEXT,
    field14 TEXT,
    field15 TEXT,
    field16 TEXT,
    field17 TEXT,
    field18 TEXT,
    field19 TEXT,

    field20 TEXT,
    field21 TEXT,
    field22 TEXT
);

getTest:
SELECT * FROM dbTest;

and here is the generated TestQueries.kt:

interface TestQueries : Transacter {
  fun <T : Any> getTest(mapper: (
    field1: String?,
    field2: String?,
    field3: String?,
    field4: String?,
    field5: String?,
    field6: String?,
    field7: String?,
    field8: String?,
    field9: String?,
    field10: String?,
    field11: String?,
    field12: String?,
    field13: String?,
    field14: String?,
    field15: String?,
    field16: String?,
    field17: String?,
    field18: String?,
    field19: String?,
    field20: String?,
    field21: String?,
    field22: String?
  ) -> T): Query<T>

  fun getTest(): Query<DbTest>
}

However, if a 23rd parameter is added:

CREATE TABLE dbTest(
    field1 TEXT,
    field2 TEXT,
    field3 TEXT,
    field4 TEXT,
    field5 TEXT,
    field6 TEXT,
    field7 TEXT,
    field8 TEXT,
    field9 TEXT,
    field10 TEXT,

    field11 TEXT,
    field12 TEXT,
    field13 TEXT,
    field14 TEXT,
    field15 TEXT,
    field16 TEXT,
    field17 TEXT,
    field18 TEXT,
    field19 TEXT,

    field20 TEXT,
    field21 TEXT,
    field22 TEXT,
    field23 TEXT
);

getTest:
SELECT * FROM dbTest;

there will be no method which takes a mapper generated:

interface TestQueries : Transacter {
  fun getTest(): Query<DbTest>
}

Edit: tested on
SqlDelight 1.2.2
Kotlin 1.3.61

@benasher44
Copy link
Contributor

Oh dang. Didn't realize this still need SQLDelight changes.

@adenisyuk
Copy link

Any chance it'll be fixed soon?

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