Kotlin extensions not useful with Kotlin codegen

[pschichtel]

The kotlin code generator generates pretty much all columns as nullable types (to be consistent with SQL), however the extensions in this project are only on non-nullable field types. So in addition to the Field<JSON> extension, Field<JSON?> extensions would also be necessary.

[t9t]

Hi, thanks @pschichtel ! We discussed it in #12 whether it would be useful or not, to use eg. JSON? but decided not to. However, I didn't realise that the Kotlin code generator generates nullable types (in fact, I didn't even know that there was a Kotlin code generator!).

I'll play with the Kotlin code generator and make sure it's included in the integration tests of this project as well. Even though it's a small fix, unfortunately I can't give you an exact time window, but I hope I will be able to address this in the coming week.

[pschichtel]

Yeah, the generator got introduced in 3.14, so pretty recent.

No rush with this, currently we are covered with 2 extensions we have in our project, mainly thought about including this lib for a more complete coverage.

[pschichtel]

In addition to this, adding @NotNull annotations where the result is known to be not null, so the kotlin compiler can infer a non-nullable type instead of a platform type, would be great.

[t9t]

I'm having a jab at this now, and it proves more troublesome than I thought. I thought an extension function on Field<JSONB?> could also be used on Field<JSONB> but apparently this is not the case. So the way it's defined right now means it's not compatible with the code that the jOOQ code generator generates, but if I change it to Field<JSONB?> it won't be compatible with code that creates a field manually (some of which may already be out there, no idea).

On top of that, there is the return value type. Preferably, I would like to match the input type, ie. Field<JSONB?>.fieldByKeyText should return Field<String?> while a Field<JSONB>.fieldByKeyText should probably return a Field<String> to match the input.

Unfortunately, I can't just declare both functions with the same name:

fun Field<JSONB?>.arrayElement(index: Int): Field<JSONB?> = JsonbDSL.arrayElement(this, index)
fun Field<JSONB>.arrayElement(index: Int): Field<JSONB> = JsonbDSL.arrayElement(this, index)

That will generate the following error:

Platform declaration clash: The following declarations have the same JVM signature (arrayElement(Lorg/jooq/Field;I)Lorg/jooq/Field;): 
public fun Field<JSONB>.arrayElement(index: Int): Field<JSONB> defined in com.github.t9t.jooq.json.jsonb in file JsonbDSL.kt
public fun Field<JSONB?>.arrayElement(index: Int): Field<JSONB?> defined in com.github.t9t.jooq.json.jsonb in file JsonbDSL.kt

@davinkevin Do you have any views on this matter? Are you already using the Kotlin extension functions you introduced in production code for example, experiencing any compatibilities issues with the jOOQ Kotlin code generator and this library.

[pschichtel]

A simple solution for the conflicting definitions would be to define them in separate packages. e.g. com.github.t9t.jooq.json.jsonb and com.github.t9t.jooq.json.jsonb.nullable. Still doesn't solve the heavy duplication though

[t9t]

I had another look at the Kotlin code being generated, and it looks like that a nullable type is always generated, regardless of the nullability of the table field (which I think is a good thing, because it leads to a consistent API). So data jsonb NULL and data jsonb NOT NULL both lead to Field<JSONB?>.

That means that all of the native jOOQ code is also operating (implicitly through generic type parameters) on the nullable types. For example, if you have SOME_TABLE.SOME_JSONB_FIELD.eq(...) then the eq() is going to expect a JSONB? or Field<JSONB?> value as well.

So then it does not make sense that for example the toField() function in this library returns a value of type Field<JSONB>, as it will not really be usable with the native jOOQ APIs at all.

My conclusion then is that I will change all of the input and return types to Field<JSON?> and Field<JSONB?>, even if it's known that the value is not nullable. Just so that it can interact with the jOOQ APIs well.

This does mean however that it will be a breaking change, because I have released the previous Kotlin extension functions as a stable version already. So unfortunately that will lead to a 2.0.0 version of the library. I will have to document this well in the readme and changelog, so that people will not be too scared of doing a "major upgrade" of the lib.

[t9t]

I would appreciate some feedback on #14 before I merge & release it. :)

[t9t]

I have merged the change and released it as version 2.0.0. Thanks for reporting this problem @pschichtel!

[davinkevin]

Sorry for the response delay, I have some important thing to deal in my personal life, so I was out of mail/github lately.

I didn't use the kotlin generator (yet), so I didn't "see" this incompatibility. For information, there is an issue to reduce this "nullability" in JOOQ repository jOOQ/jOOQ#10212.

Your PR @t9t is quite good, thanks for the modification and again, sorry to not answer in time.

[t9t]

No worries @davinkevin! Let me know if you happen to encounter any issues in the future and I'll be happy to assist. Take care.