-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Argument name based constructor auto-mapping #2196
Argument name based constructor auto-mapping #2196
Conversation
When there are multiple constructors, users must use `@AutomapConstructor`. Although it would be possible to implement find-the-best-match type logic,
# Conflicts: # src/main/java/org/apache/ibatis/executor/resultset/DefaultResultSetHandler.java # src/main/java/org/apache/ibatis/session/Configuration.java # src/test/java/org/apache/ibatis/builder/XmlConfigBuilderTest.java # src/test/java/org/apache/ibatis/builder/xsd/XmlConfigBuilderTest.java
# Conflicts: # src/main/java/org/apache/ibatis/session/Configuration.java # src/site/es/xdoc/configuration.xml # src/site/ja/xdoc/configuration.xml # src/site/ko/xdoc/configuration.xml # src/site/xdoc/configuration.xml # src/site/zh/xdoc/configuration.xml
a9451fc
to
7a76f77
Compare
…perty-automapping This logic is applied only to the new arg-name-based constructor auto-mapping. Although it is possible (and it may also be the expected behavior) to apply the same logic to the old column-order-based constructor auto-mapping, it will break many existing solutions, probably.
@kazuki43zoo @jeffgbutler @hazendaz I plan to merge this soon. |
@@ -519,6 +525,10 @@ private List<UnMappedColumnAutoMapping> createAutomaticMappings(ResultSetWrapper | |||
if (autoMapping == null) { | |||
autoMapping = new ArrayList<>(); | |||
final List<String> unmappedColumnNames = rsw.getUnmappedColumnNames(resultMap, columnPrefix); | |||
List<String> mappedInConstructorAutoMapping = constructorAutoMappingColumns.remove(mapKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harawata Might be useful for a comment here to understand why its removing vs others getting data. I think I grasp it but find it somewhat confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @hazendaz !
Added a brief comment.
The entry is used only once, so there is no point in keeping it on memory.
An attempt at fixing #2192
argNameBasedConstructorAutoMapping
is added.Compared to the current (=column order based) constructor auto-mapping, name-based auto-mapping is more lenient.
Cc: @h3adache