Hello,
If we use NamedParameterJdbcOperations.queryForObject with RowMapper, the RowMapper will call method mapRow for every row and the result (every row) will be add into a List.
Then if we have more than one rows , DataAccessUtils.nullableSingleResult will throw an exception.
if (CollectionUtils.isEmpty(results))
{ *throw new EmptyResultDataAccessException(1);* }
if (results.size() > 1)
{ *throw new IncorrectResultSizeDataAccessException(1, results.size())*; }
But If our results contains a lot of rows (example 10 000 rows), we have to wait to fetch all 10 000 rows to throw IncorrectResultSizeDataAccessException.
I think this is too slow.
It's better to throw the exception on second row.
Comment From: nkjackzhang
You can write your sql with "limit 1" to make sure containing at most 1 row.
Comment From: sbrannen
The Javadoc for variants of queryForObject(...) that accept a RowMapper states the following:
Throws:
org.springframework.dao.IncorrectResultSizeDataAccessException- if the query does not return exactly one row, or does not return exactly one column in that row
So I think that is clear regarding the current behavior.
But If our results contains a lot of rows (example 10 000 rows), we have to wait to fetch all 10 000 rows to throw
IncorrectResultSizeDataAccessException.
If the SQL query you supply selects more than one row, all of the rows will be fetched from the database, and Spring cannot do anything to prevent that.
It is therefore the developer's job to modify the query so that it only selects a single row.
As @nkjackzhang pointed out, you can do that by limiting the results using syntax supported by your database.
Note, however, that a RowMapperResultSetExtractor is used behind the scenes for extracting the data from the ResultSet and applying the provided RowMapper. The RowMapperResultSetExtractor is instantiated with a value of 1 for the rowsExpected constructor argument for variants of queryForObject(...) that accept a RowMapper. The value of rowsExpected is currently only used to influence the initial size of the local ArrayList; however, the code could be modified to throw an exception if the number of rows in the ResultSet exceeds the value of rowsExpected before continuing with processing of the additional rows. Doing so would short circuit the algorithm and void unnecessary invocations of the supplied RowMapper (in case those operations are costly).
@jhoeller, what do you think about making such a change to RowMapperResultSetExtractor?
Comment From: sbrannen
Tentatively slated for 5.2 M1 for further consideration.
Comment From: LaCoCa
Yes but if want to catch not data found (EmptyResultDataAccessException) and to many rows (IncorrectResultSizeDataAccessException), limit 1 can throw only EmptyResultDataAccessException.
Comment From: sbrannen
Yes but if want to catch not data found (EmptyResultDataAccessException) and to many rows (IncorrectResultSizeDataAccessException), limit 1 can throw only EmptyResultDataAccessException.
Well, if you want to avoid fetching all of the unexpected rows, you can always first perform a query that selects a count(...) of such rows and then handle any error (i.e., too few / too many rows) before actually performing the query that selects the data from the rows.
Comment From: LaCoCa
Yes we can modified the sql statements but the problem is that a lot developers think that queryForObject work as I expect :). Also we have a lot of already done sql statements which have to modified.
So, if possible, make a fix .
Comment From: sbrannen
Yes we can modified the sql statements but the problem is that a lot developers think that queryForObject work as I expect :).
Then that would rather be an issue regarding educating users. Thus it may perhaps be beneficial to improve the documentation.
Also we have a lot of already done sql statements which have to modified.
Based on my experience, you need to do that anyway.
Rationale: Spring will not modify your SQL query to insert support for limiting the number of rows returned from the database. An O/R mapping framework (e.g., JPA, Hibernate, etc.) may be able to do that for you, but Spring's JdbcOperations support does not. Thus, the entire ResultSet will be populated in-memory for all rows in the results returned from the database, which is transported over the network (unless you're using an in-memory database).
In other words, for the sake of performance (network overhead, etc.) it is in your best interest to ensure that your queries select as little data as possible.
Whether or not Spring introduces a "short circuit" mechanism to avoid unnecessary invocations of your RowMapper would therefore likely only result in a minor performance improvement.
So, if possible, make a fix .
As I don't see this as a bug, I don't think there is anything to fix per se; however, I think we should leave this issue open to see if additional members of the community would also find such an enhancement (as proposed in https://github.com/spring-projects/spring-framework/issues/22651#issuecomment-476164408) worthwhile.
Comment From: jhoeller
Since it looks like we'll keep the existing behavior and put the onus on the provided SQL, I'm closing this issue here.