Description
I think we have a design problem in the framework
First, there is DataAccessStrategy
, which acts like an orchestrator in general - it is responsible to delegate the SQL creation, construction of the ParameterSource
for the SQL, and actual execution of the query via spring-jdbc
module api. Then we have an EntityRowMapper
to map the result of the ResultSet
to POJO.
The problem is that EntityRowMapper
(as well as MapEntityRowMapper
) is calling findAllByPath
of the DataAccessStrategy
to fetch the dependencies of an aggregate. It is at least a circular depenedency between logic layers, and it brings up a question (even if we will not take into consideration the issue raised in #1434 and we will go with N SELECT queries instead of simple joins):
Do we really understand that the entity has a relation by the time of getting a response from the database? No. But that's what actually the code is doing right now - we check for the relation existence in the ReadingContext#resolveRelation
method and at that time, we are creating second SQL query (why we do it here, and not earlier?) and fetching the request. Instead we can choose to not execute the second SELECT request in some rare OneToOne mapping cases (for instance we have OneToOne mapping and the id of child aggregate in the root entity is already absent, so there is no need to execute second SELECT - we will not find anything).
So I think we should clearly separate the layers, like:
- creation of SQL to fetch all of the data that we need (no matter by what means, select. subselect, join or whatever it is) - we have the relationship information before the actual requests to the DB. Moreover, if you are concerned about the performance for the construction of the possible second SQL request - you can cache it, because fetching the child aggregate from the parent will be done the same (for example now it is a second select - it will be the same, just root id different, but it does not matter).
- Creation
ParameterSource(-es)
for the SQL created on the previous step. - The actual execution layer, seems delegation to spring-jdbc is just fine
- And then the mapping by
RowMapper
s
I think there is a lot of work required for this to be done, so maybe we should consider to decompose this task. In any case, I really think this issue should be taken into account for the future of the project.