Skip to content

Deprecate rowsExpected property of SqlQuery #34512

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

Closed
quaff opened this issue Feb 28, 2025 · 5 comments
Closed

Deprecate rowsExpected property of SqlQuery #34512

quaff opened this issue Feb 28, 2025 · 5 comments
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement

Comments

@quaff
Copy link
Contributor

quaff commented Feb 28, 2025

SqlQuery::setRowsExpected is invoked somewhere, but it make no sense since SqlQuery::getRowsExpected is never invoked anywhere.

We have two options:

  1. use it for query.
  2. deprecate it for removal.

See GH-34502

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 28, 2025
@bclozel
Copy link
Member

bclozel commented Feb 28, 2025

I still don't get it. This is a java-bean style property on this class. While it's not being used in Spring Framework I think it's meant to be used by applications. Do you have any replacement for this to suggest for people using this?

This is not my area of expertise so I'll leave it to other team members to decide on this.

@quaff
Copy link
Contributor Author

quaff commented Feb 28, 2025

Do you have any replacement for this to suggest for people using this?

I believe it's misused here, the intention of rowsExpected is for performance consideration of creating ArrayList, shouldn't rely on it.
If we decide to keep it, then we should use it for initialCapacity of ArrayList.

public List<T> extractData(ResultSet rs) throws SQLException {
List<T> results = (this.rowsExpected > 0 ? new ArrayList<>(this.rowsExpected) : new ArrayList<>());
int rowNum = 0;
while (rs.next()) {
results.add(this.rowMapper.mapRow(rs, rowNum++));
}
return results;
}

@sbrannen sbrannen added the in: data Issues in data modules (jdbc, orm, oxm, tx) label Mar 1, 2025
@sbrannen sbrannen changed the title "rowsExpected" of SqlQuery is not actually used rowsExpected property of SqlQuery is not actually used Mar 1, 2025
@sbrannen
Copy link
Member

sbrannen commented Mar 1, 2025

Do you have any replacement for this to suggest for people using this?

I see it the same way as @quaff.

The rowsExpected property cannot be relied on in any way. It is not used to enforce anything. Rather, it's only meant for potentially efficient storage in case the user knows in advance how many rows will be returned, as stated in the Javadoc for setRowsExpected().

Set the number of rows expected.

This can be used to ensure efficient storage of results. The default behavior is not to expect any specific number of rows.

This is analogous to the rowsExpected property of RowMapperResultSetExtractor:

rowsExpected the number of expected rows (just used for optimized collection handling)

The difference is that rowsExpected is used in RowMapperResultSetExtractor, but it is not used in SqlQuery.

Thus, as @quaff pointed out, with the status quo there is not much point in having rowsExpected in SqlQuery.

@sbrannen sbrannen added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Mar 1, 2025
@jhoeller
Copy link
Contributor

jhoeller commented Mar 2, 2025

Let's deprecate those methods for 6.2.4 if they don't have a clear purpose.

@sbrannen sbrannen removed status: waiting-for-triage An issue we've not yet triaged or decided on status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Mar 2, 2025
@sbrannen sbrannen self-assigned this Mar 2, 2025
@sbrannen sbrannen added this to the 6.2.4 milestone Mar 2, 2025
@sbrannen sbrannen added the type: enhancement A general enhancement label Mar 2, 2025
@sbrannen sbrannen changed the title rowsExpected property of SqlQuery is not actually used Deprecate rowsExpected property of SqlQuery Mar 2, 2025
quaff added a commit to quaff/spring-framework that referenced this issue Mar 3, 2025
quaff added a commit to quaff/spring-framework that referenced this issue Mar 3, 2025
@sbrannen
Copy link
Member

sbrannen commented Mar 3, 2025

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Mar 3, 2025
@sbrannen sbrannen removed this from the 6.2.4 milestone Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants