DATAJDBC-113 - Added support for Set-valued references. #15
Conversation
When loading the referenced entities get loaded by a separate select statement. Changes to a collection get handled just like 1-1 relationships by deleting all and reinserting them again. Introduced a separate method in JdbcPersistentProperty instead of just unsing the table name, although it currently still is the table name. Now providing SqlTypes in SqlParameterSource, since MySql has a problem with null parameters when it doesn't know the type. Fixed an error where a SQL-statement generated by the SqlGenerator was obviously wrong.
| private final JdbcPersistentProperty idProperty; | ||
|
|
||
| @java.beans.ConstructorProperties({ "entity", "conversions", "context", "template" }) | ||
| public EntityRowMapper(JdbcPersistentEntity<T> entity, ConversionService conversions, JdbcMappingContext context, |
There was a problem hiding this comment.
What does @ConstructorProperties do for you? I read the javadocs, and didn't get it. I commented out both usages, and all the test cases still passed, so ??? to me.
There was a problem hiding this comment.
Good question /head scratch/ I thought Olli introduced that at some point, but he didn't. At least not in this class. It might be that lombok created it when delomboking the code.
Anyway I agree: It seems to superfluous and will remove it.
| @NonNull private final String prefix; | ||
|
|
||
| @java.beans.ConstructorProperties({ "resultSet", "conversionService", "prefix" }) | ||
| private ResultSetParameterValueProvider(ResultSet resultSet, ConversionService conversionService, String prefix) { |
|
|
||
| String tableColumns = String.join(", ", propertyNamesForInsert); | ||
| String parameterNames = propertyNamesForInsert.stream().collect(Collectors.joining(", :", ":", "")); | ||
|
|
|
|
||
| Class<?> type = p.getType(); | ||
| if (Collection.class.isAssignableFrom(type)) | ||
| return collectionPropertyAsStream(p, propertyAccessor); |
There was a problem hiding this comment.
I prefer wrapping these single line statements with braces.
| if (Collection.class.isAssignableFrom(type)) | ||
| return collectionPropertyAsStream(p, propertyAccessor); | ||
|
|
||
| return singlePropertyAsStream(p, propertyAccessor); |
There was a problem hiding this comment.
Don't know if it's just me, but I prefer wrapping this bare return statement in an else so it's clear its part of the previous statement, and nothing can sneak in in the future accidentally breaking the semantics of this either/or outcome.
There was a problem hiding this comment.
I actually disagree on this one. With the final 'return' I find it much easier to see that the bucket stops here.
I'll poll the team to see if we have a de facto style guide for this.
There was a problem hiding this comment.
Nobody seems to have a strong opinion but the single return statement seems to be preferred.
Olli pointed out the ternary operator as an alternative for single returns and I like that even more. So I want with that here and in the following instance.
| return Stream.empty(); | ||
| } | ||
|
|
||
| return ((Collection<Object>) property).stream(); |
| public class JdbcUtil { | ||
|
|
||
| private static final Map<Class, Integer> sqlTypeMappings = new HashMap<>(); | ||
|
|
There was a problem hiding this comment.
Should the key type be Class<?>?
There was a problem hiding this comment.
Good question. I thought about that and came to the conclusion that Class<?> would mean that there is a single unnamed type ? that applies to all keys. But the keys are obvious of type Class<T> with a different T for each key so Class is actually the best type we can correctly specify.
Does that make sense?
There was a problem hiding this comment.
I did some tests and seems like you are right and I'm confused :-/ https://stackoverflow.com/q/46050311/66686
| } | ||
|
|
||
| public static int sqlTypeFor(Class type) { | ||
| return sqlTypeMappings.keySet().stream() // |
|
|
||
| private Object getObject(String column) { | ||
|
|
||
| return values.get(index).get(column); |
There was a problem hiding this comment.
Don't think you need the extra blank line here.
Applied changes from review. Removed superfluous @ConstructorProperties annotations. Fixed formatting. Improved usage of generics.
We now expose a bind marker API that allows to model a vendor-specific bind marker strategy. The currently supported database systems (Postgres, MsSql) implement indexed respective named strategies that differ in how placeholders and identifiers are generated. Original pull request: #19.
Improve Javadoc. Rename parameters for more expressiveness. Replace ASCII filtering with filter Function to move actual filtering into dialects. Original pull request: #19.
2 participants
When loading the referenced entities get loaded by a separate select statement.
Changes to a collection get handled just like 1-1 relationships by deleting all and reinserting them again.
Introduced a separate method in JdbcPersistentProperty instead of just using the table name, although it currently still is the table name.
Now providing SqlTypes in SqlParameterSource, since MySql has a problem with null parameters when it doesn't know the type.
Fixed an error where a SQL-statement generated by the SqlGenerator was obviously wrong.