Open schauder opened 2 years ago
The current process of BasicJdbcConverter
for writing a value is:
=> the sql-type determined in the beginning does not match the target type T1, resulting in whole bunch of errors.
Side problem: null values don't get converted by Spring Framework converters
Better process:
Jens @schauder , I think I can try to make it work so, I have already invested some time in order to understand the guts of this process, so I think it will make sence. May I take a look at it?
Hi Mikhail,
of course you may take a look! But as a fair warning: This is a pretty central part of Spring Data JDBC and it is perfectly possible, that we have some ideas that we don't have properly documented yet, which could lead to a failure to accept an otherwise good PR.
If you are fine with that risk, please go ahead. And please let us know if you do so we can leave it to you at least for some time.
Yes, I think I am perfectly fine, I will try to fix this issue :) If I there will be some arguable questions during implementation, I will try to reach you out :)
Things also to consider:
@Query
annotation) vs those where we do (Query derivation).At least it helps me understand why there are two conversions. See #1829
This became obvious when analysing #1127 Part of this problem is hopefully fixed by a fix for #1089.
We have multiple checks for determining what datatype a value should get converted to and I don't think they add to something that I understand.
BasicRelationalConverter.writeValue(@Nullable Object value, TypeInformation<?> type)
converts the value twice when it is a simple type and only once otherwise. At the very least this should be refactored so I can understand why it is doing that.Concept for a rewrite of the writing conversion logic.
The goal is to have a clean and conceptual consistent process for converting Java values to database values.
The process should look like this:
Identify the conversion target The conversion target is a Java type + a java.sql.SQLType (typically a java.sql.JDBCType)
perform the conversion.
The process for identifying the conversion target should follow this process:
If it is an entity we don't convert it.
If it is collection-like, the target is the same kind of collection. We run the same process to determine the elements of the collection.
If there is a matching
WritingConverter
it determines the target.If the incoming type in question is a simple type, the target type is just that type.
Otherwise we apply the standard conversions (e.g. Enums get converted to Strings ... we need to inspect what is currently in the source code). I wonder if we can identify the conversion target without actually applying it in this step.
there is special handling for arrays ... we need to slot them in somehow. I currently don't think they need special consideration in the conversion process, but I might be wrong about this.
Once the target type is determined the SQLType gets derived from that. This is either a lookup from
JdbcUtil
or it is part of the conversion target if it isJdbcUtil
. I guess that means we don't have aSQLType
, but aFunction<Object, SQLType
.Part of the challenge and why the current implementation is so convoluted is, that we a have different sets of information going into this process:
save
operations we do have the property.The class structure should lean heavily on Cassandra, but with a combination of Java type + SQLType instead of just a Java type as the target.
https://github.com/spring-projects/spring-data-cassandra/blob/main/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/ColumnType.java https://github.com/spring-projects/spring-data-cassandra/blob/main/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/ColumnTypeResolver.java https://github.com/spring-projects/spring-data-cassandra/blob/main/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/DefaultColumnTypeResolver.java https://github.com/spring-projects/spring-data-cassandra/blob/main/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/MappingCassandraConverter.java
Related issues
The following issues might either get fixed as a side effect of this issue or much easier to fix after this issue is resolved.
https://github.com/spring-projects/spring-data-relational/issues/787 https://github.com/spring-projects/spring-data-relational/issues/1023 https://github.com/spring-projects/spring-data-relational/issues/725 https://github.com/spring-projects/spring-data-relational/issues/629