https://kotlinlang.org logo
Join Slack
Powered by
Hi, everyone. Komapper v1.6.1 is out. We support M...
# komapper
t
Hi, everyone. Komapper v1.6.1 is out. We support MariaDB R2DBC and MySQL R2DBC from this version. Please give it a try. https://github.com/komapper/komapper/releases/tag/v1.6.1
🎉 1
d
I added my own converter for it for now... but it would be nice to be able to map multiple sql types to the same kotlin primitive... like with my suggestion using a parameter in the 
@KomapperColumn
annotation or some other annotation?... I think it could simplify the api, since I'm sure users would prefer to use String's for text or clob fields unless they have some field that can't be contained in a String...
t
You’re missing the mysql TEXT type
I tried to map MySQL text type to io.r2dbc.spi.Clob, but jasync-r2dbc-mysql does not support io.r2dbc.spi.Clob.
d
It supports text
I have:
Copy code
class LongTextTypeR2dbc : R2dbcUserDefinedDataType<LongText> {

    override val name: String = "text"
    override val r2dbcType: Class<*> = String::class.javaObjectType

    override val klass: KClass<LongText> = LongText::class

    override fun getValue(row: Row, index: Int): LongText? {
        return row.get(index, String::class.javaObjectType)?.let { LongText(it) }
    }

    override fun getValue(row: Row, columnLabel: String): LongText? {
        return row.get(columnLabel, String::class.javaObjectType)?.let { LongText(it) }
    }

    override fun setValue(statement: Statement, index: Int, value: LongText) {
        statement.bind(index, value.value)
    }

    override fun setValue(statement: Statement, name: String, value: LongText) {
        statement.bind(name, value.value)
    }

    override fun toString(value: LongText): String {
        return value.value.toString()
    }
}
Copy code
data class LongText(val value: String)
Which is why I think adding this feature to map multiple types to Strings would be great!
Instead of having a wrapper for each type, or using native versions which aren't always so user friendly...
Or at least allowing to use value classes for user defined types...?
t
it would be nice to be able to map multiple sql types to the same kotlin primitive... like with my suggestion using a parameter in the @KomapperColumn annotation or some other annotation?
The solution with annotations has the weakness that it cannot be used with Template Query. So we want to be cautious about adding new annotations.
d
Yeah... there could be many ways to do this... I just suggested a few.
Why should this make a difference in template queries?
You're not doing any automatic mapping there (although I did think that it might be nice...), there doesn't even seem to be any reference to the entity object there...
t
Why should this make a difference in template queries?
This is because the result of Template Query execution is not the data structure of an entity. In other words, Komapper cannot use information from the entity metamodel when converting Template Query results to Kotlin data types. On the other hand, user defined data type is also used in Template Query.
d
On the other hand, user defined data type is also used in Template Query
??
But that has nothing to do with that annotation that's on the Entity... there could be a default user defined type, and the ones that need an annotation...
The default could be used by Template Query (although I don't know the code well enough to understand where...), and the entities could be allowed to "decide" which type to use.
t
My point is that user defined data types are more versatile than type conversion using annotations. In order to keep Komapper simple, I believe it is best to avoid adding new annotations as much as possible.
d
True that type conversion was probably made for this purpose... But it's a waste to get a clone, just to convert it to a string after... For domain classes, I'd agree that mappers are probably better since the underlying r2dbc or jdbc API can't produce them, but it's more efficient to request a string straight from the underlying implementation. Especially in such cases like jsync that don't even support clobs... Then you're stuck that you can't support text fields at all there which is quite limiting.
t
You can override primitive type mappings with user defined data types. How about trying this method?
d
In jasync there's no primitive type for text to override
And in other implementations clobs are harder to work with and wasteful to create when you can just get a string from the driver instead
I posted my current implementation using a data class wrapper for text, it would just be nice to be able to not have to wrap it. I guess that it's not so simple to implement though... So I could live with the wrapper if you don't think my suggestion is possible...
Though I would maybe at least document the possibility to add such wrappers when drivers don't support certain field types...
Text is such a basic type, it could turn down potential users...
If you don't like annotations, maybe allow using `value class`s for user defined types? At least it could save boxing...
Then, you'd try to look up a type, if not, then you'd use the underlying type in the value class
Then you could use them in the dialects for types like TEXT... so that users don't have to implement them themselves...
t
I am gradually understanding. Let me think about it for a while.
d
Another consideration would be that someone implementing a user defined type has currently no way to denote which dialect it relates to... And how to implement different ones for different dialects... So that could also be considered as part of this question...
Any new thoughts on this @Toshihiro Nakamura? Thanks for considering all these suggestions!
t
I will support to map multiple sql types to the same kotlin primitive. However, we want to limit it to specific types. For now we will only support Clob and String. Other types will be supported upon request. You can write as followings in the next version.
Copy code
@KomapperColumn(mapping = MappingType.CLOB_STRING)
val name: String
d
Why not have 
MappingType.CLOB_STRING
contain a KClass that does the transformation, and allow the user to specify their own?
t
This is because Komapper does not know how to convert user-specified types. If we allow user-specified types, you must also specify the class that does the conversion, but that would be an ugly API.
Further, we do not want the entity to rely on Jdbc or R2dbc specific converters.
d
Further, we do not want the entity to rely on Jdbc or R2dbc specific converters.
Maybe not the converter, but maybe a provider class that provides the appropriate converter? In general, someone writing a converter is making their entity depend on such a specific converter and nothing forces them to write both... but I guess you mean having that reference in the entity declaration... which a provider would solve. But I see your point, that it might not be the nicest api... json serializers/deserializers use this kind of method for custom json converters per field... it does provide a lot of power though...
The only funny thing I think of having that on 
@KomapperColumn
is that this is really specific to text/clob fields... not integers or any other types...?
For enums you have 
@KomapperEnum
...
Maybe 
@KomapperText(mapping=...)
?
t
this is really specific to text/clob fields... not integers or any other types...?
No. Other types may be supported in the future. For example:
Copy code
@KomapperColumn(mapping = MappingType.BLOB_BYTES)
val file: ByteArray
d
So if a type that isn't valid for a particular field is used, then it'll be a compile time error?
t
Yes, exactly.
d
So in the docs, you'd just specify which MappingTypes are supported for each field type in each dialect...?
t
Yes, I think so. By the way, MappingType.CLOB_STRING will be supported for all dialects.
d
In Jasync it's not CLOB_STRING, it's TEXT_STRING 🙈?
But then, maybe R2dbc considers that a CLOB? I wonder if R2DBC's getClob can handle such a field...
In the underlying driver, it would only use String... so it would still be limited to the maximum String size, but maybe getClob streams it? It would make a difference for those trying to port from mysql to postgres for example or from jasync to another driver, if there will eventually be one...
In mysql, there really is NO Clob only TEXT
t
In Jasync it’s not CLOB_STRING, it’s TEXT_STRING 🙈?
Yes. CLOB_STRING means that Komapper map SQL TEXT to Kotlin String in Jasync.
We have not implemented any error handling, but have confirmed that it works with all supported databases. See if you are interested. https://github.com/komapper/komapper/pull/848
d
Yeah, that would do it. And it doesn't have to be retreived as a clob and then converted to string... I guess making something more generic would be much harder.
t
Does the above PR not meet your requirements? If not, I will close the PR.
Or at least allowing to use value classes for user defined types...?
Yes, you can use value classes. You can rewrite the LongText data class as a value class in your example above.
Copy code
@JvmInline
value class LongText constructor internal(val value: String)
If a value class has a non-public constructor, user-defined type can handle it.
d
Interesting... that might be a good thing to document! When I tried, I used a public constructor and it ignored my type... The PR does meet my current requirements... I'm just wondering if it could be made a bit more powerful and generic.
The good thing about the PR is that it doesn't just make it impossible to use a TEXT field in Jasync's r2dbc driver... which is itself a big improvement for other potential users.
t
I’m just wondering if it could be made a bit more powerful and generic.
I have a new idea. Users must do 3 things to map multiple sql types to the same kotlin type. 1. Create a user defined type. In your case, it means that LongText and LongTextTypeR2dbc. 2. Create a mapping object as follows:
Copy code
object LongTextMapping<String, LongText> : DataTypeMapping {
  fun exteriorToInterior(exterior: String) = LongText(exterior)
  fun interiorToExterior(interior: LongText) = interior.value 
}
3. Specify the above mapping object to `@KomapperColumn`:
Copy code
@KomapperColumn(mapping = LongTextMapping::class)
val decription: String
Typical use cases such as mapping SQL TEXT to Kotlin String can be supported by Komapper. In that case, steps No. 1 and 2 are not necessary.
d
How would you know in the underlying layer whether to use 
getClob(...)
or 
get(..., String::class)
from say, r2dbc?
Oh... from 
LongTextTypeR2dbc
...
So why that extra mapping object? Why not just specify 
LongTextTypeR2dbc
as the mapping in the 
@KomapperColumn
?
(if really needed technically by komapper it might be generated by ksp?)
Ok, I think I get it... you're trying to avoid implementation-dependent types on entities...
But I still think that 
LongTextMapping
might be a bit of boilerplate since 
LongText
is really just a 
value class
thats' real value is a 
String
... also it seems like a 
DataTypeMapping
IS a 
DataTypeConverter
that allows mapping back to a primitive type on a specified column... even if it would be required, why not merge the two concepts and be able to use them both ways? To the class without an annotation (perhaps with an option to use another converter to that same class), and to the primitive type with an annotation?
In general, this idea sounds more powerful and versatile than the last one, but if we work it out a little bit more, it might get even better 😃 !
t
OK. If you accept some restrictions, you can reduce boilerplate. 1. Create a wrapper class as a value class. That class must have a public constructor and a public property:
Copy code
@JvmInline
value class LongText(value: String)
2. Crate a user defined data type that uses the above wrapper class:
Copy code
class LongTextTypeR2dbc : R2dbcUserDefinedDataType<LongText> { ... }
3. Specify the wrapper class to @KomapperColumn:
Copy code
@KomapperColumn(wrapper = LongText::class)
val description: String
d
How do you see that as a restriction compared to the previous proposition?
That could actually solve the problem pretty elegantly... but how could this be explained in the docs? Maybe instead of having (on the dialects page):
Copy code
Kotlin Type	Database Type
java.math.BigDecimal	NUMERIC
java.math.BigInteger	NUMERIC
java.sql.Array	ARRAY
java.sql.Blob	BLOB
java.sql.Clob	CLOB
java.sql.NClob	CLOB
java.sql.SQLXML	CLOB
maybe adding an extra column for 
value class
wrappers that could also be independent of jdbc or r2dbc (like using String instead of CLOB) and can be used as "wrappers" (is that the right word for this?)... I'm just thinking out loud about how other users would understand this new concept, so that it can be made in a way that's easy to understand/use...
Maybe you've already thought of this though...
That's for the provided "wrappers"... Though, it might be a bit difficult to understand why that's a "wrapper"... the wrapper is just an implementation detail inside the library. Maybe it might be more appropriate to use "typeWrapper"?
Then in the docs you could just list the available type wrappers for the types, and which ones are the default for which dialect.
t
I certainly don’t feel comfortable with the word “wrapper”.
Another option is “interiorType”.
d
That's a possibility... but then 
Clob::class
is also an interiorType, which I don't suppose could be used with such an annotation... only the "alternateType"s that are defined...
So there are the regular user defined types that can't be used in such an annotation and are the default representation of that db type, and there's these new alternate types... that CAN be used in such annotations...
They also have the name "user defined types" which could be confusing.
t
Thanks. “alternateType” is good.
d
I wonder if it might be too much... but maybe having an annotation like:
Copy code
@JvmInline
@KomapperAlternateType(["text", "CLOB"])
value class LongText(value: String)
could possibly have ksp generate the custom type, and at the same time allow validation of what can be used in the 
@KomapperColumn(alternateType=?)
...?
Then the user wouldn't be able to get confused between the two concepts..
And there would be 0 boilerplate...
Or maybe instead of using the db types... have a list of the types supported by default:
Copy code
@JvmInline
@KomapperAlternateType(["io.r2dbc.spi.Clob", "java.sql.Clob"])
value class LongText(value: String)
which would make a single facade for alternate types of any implementation...
I'm not sure which would be better, though... and the current default implementations would need to be augmented with extra types they can map to with the second option...
t
could possibly have ksp generate the custom type,
That is almost impossible. There are special cases of JDBC/R2DBC APIs that are not suited for automatic generation.
d
So maybe just for validation purposes...? Ksp could make sure that there's a custom type for that value class, and only such a value class can be used in the KomapperColumn annotation?
t
Komapper KSP processor does not make sure that there’s a custom type(user defined data type) for that value class. That is the user’s responsibility. It is difficult for Komapper KSP processor to generate the user defined data type, because Komapper does not know all the ways to map SQL types to Kotlin types.
d
I guess if you manage to provide enough alternate types out-of-the-box, users won't usually need to write them anyways, just as long as they're documented...
These out-of-the-box alternate types could also save the user from having their entities tied down to r2dbc or jdbc or a certain dialect... all those implementations could use the same value class as the alternate type.
The end result might usually be a kotlin primitive anyways.
Wow! I think this could be a great feature...! I didn't like from the start that I needed to use Clobs, etc... and that they're back-end specific! The only thing left would be if there were a way for the USER to define what the default types should be for there project so that they don't have to pollute all their entities with unecessary annotations?
Say if 99% of entities use String for TEXT fields and just 1% uses CLOB, it would be a waste to have to annotate 99% of the fields.
For backwards compatibility, I understand the default would have to stay the same, though.
Or maybe there could be the backward compatible defaults, and the new ones, or the user-defined ones...
t
I created a new pull request. Please give me your feedback. https://github.com/komapper/komapper/pull/856
42 Views
Open in Slack
PreviousNext
Powered by