Skip to content
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

Made BeanPropertySqlParameterSource.beanWrapper protected #28527

Closed
msangel opened this issue May 26, 2022 · 1 comment
Closed

Made BeanPropertySqlParameterSource.beanWrapper protected #28527

msangel opened this issue May 26, 2022 · 1 comment
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@msangel
Copy link

msangel commented May 26, 2022

I found BeanPropertySqlParameterSource useful for use but it still supports java 1.3 java types. Today its normal to use for temporal fields from java.time package. But BeanPropertySqlParameterSource know nothing how to handle that and no way to plug that functionality into.

@Override
	public int getSqlType(String paramName) {
		int sqlType = super.getSqlType(paramName);
		if (sqlType != TYPE_UNKNOWN) {
			return sqlType;
		}
		Class<?> propType = this.beanWrapper.getPropertyType(paramName);
		return StatementCreatorUtils.javaTypeToSqlParameterType(propType);
	}

this.beanWrapper is private so extending BeanPropertySqlParameterSource will give useless object and StatementCreatorUtils.javaTypeToSqlParameterType stuck in old java.
Making BeanPropertySqlParameterSource.beanWrapper protected will allow to override this class and manipulate with custom types of bean, including ones from java.time.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 26, 2022
@rstoyanchev rstoyanchev added the in: data Issues in data modules (jdbc, orm, oxm, tx) label Feb 9, 2023
@simonbasle simonbasle self-assigned this Mar 15, 2023
@simonbasle simonbasle added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 15, 2023
@simonbasle
Copy link
Contributor

simonbasle commented Mar 15, 2023

It looks like the necessary support for LocalTime/LocalDate/LocalDateTime was added as part of gh-28778 a few month after this issue was created. That said, the aforementioned change didn't cover two additional types from java.time: OffsetTime (a TIME_WITH_TIMEZONE) and OffsetDateTime (a TIMESTAMP_WITH_TIMEZONE).

In the light of this, I'm going to close this issue as declined because we don't really need to address the initial ask (making this.beanWrapper protected).

I'm going to open a PR to add coverage for the two missing types mentioned above, though: #30123

@simonbasle simonbasle closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2023
simonbasle added a commit to simonbasle/spring-framework that referenced this issue Mar 15, 2023
This commit adds mapping for two types from the `java.time` package,
complementing the types that are already translatable to Sql types
TIME, DATE and TIMESTAMP:
 - `OffsetTime` maps to a `TIME_WITH_TIMEZONE`
 - `OffsetDateTime` maps to a `TIMESTAMP_WITH_TIMEZONE`

This is in accordance with the B.4 table provided in the JDBC 4.2
specification.

When preparing statements, these `java.time` types use the `setObject`
method. Tests covering the 5 `java.time` classes have also been added.

See spring-projectsgh-28778
See spring-projectsgh-28527
simonbasle added a commit that referenced this issue Apr 4, 2023
This commit adds mapping for two types from the `java.time` package,
complementing the types that are already translatable to Sql types
TIME, DATE and TIMESTAMP:
 - `OffsetTime` maps to a `TIME_WITH_TIMEZONE`
 - `OffsetDateTime` maps to a `TIMESTAMP_WITH_TIMEZONE`

This is in accordance with the B.4 table provided in the JDBC 4.2
specification.

When preparing statements, these `java.time` types use the `setObject`
method. Tests covering the 5 `java.time` classes have also been added.

See gh-28778
See gh-28527
Closes gh-30123
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) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

4 participants