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

Use force-quoting in R2dbcMappingContext by default #1993

Open
ShenFeng312 opened this issue Feb 14, 2025 · 6 comments
Open

Use force-quoting in R2dbcMappingContext by default #1993

ShenFeng312 opened this issue Feb 14, 2025 · 6 comments
Assignees
Labels
status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement

Comments

@ShenFeng312
Copy link

ShenFeng312 commented Feb 14, 2025

I have a table where the field is desc. Since it's a reserved keyword in PostgreSQL, I had to use the annotation @Column("\"desc\"") to allow it to be written properly. However, when reading the data, I found that it cannot be read correctly because in org.springframework.data.relational.core.conversion.MappingRelationalConverter#read, the org.springframework.data.relational.core.conversion.RowDocumentAccessor#hasValue method doesn't recognize the field. I believe some logic should be added here to solve this issue.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 14, 2025
@ShenFeng312 ShenFeng312 changed the title MappingR2dbcConverter cannot convert reserved keyword fields MappingRelationalConverter cannot convert reserved keyword fields Feb 14, 2025
@ShenFeng312
Copy link
Author

can we add a annotation like @ResultSetKey("desc") or other way to support it @mp911de @schauder

@schauder
Copy link
Contributor

With @Column("\"desc\"") you make the double quotes part of the column name. Is that intended? SQL identifier in @Column, @Table and similar annotations are used exactly as they are given, and properly quoted to avoid interpretation as key words, or automatic changes to upper or lower case.

Therefore I guess you really want to use @Column("desc").

If that still doesn't work, please provide a full reproducer, preferably base on https://github.com/schauder/issue-jdbc-1993

@schauder schauder added the status: waiting-for-feedback We need additional information before we can continue label Feb 17, 2025
@ShenFeng312
Copy link
Author

ShenFeng312 commented Feb 18, 2025

@schauder Thank you for your help, and sorry I forgot to mention that this bug occurs under r2dbc. I tried using the demo provided in your repository for testing, but I’m not sure how to use TestcontainersConfiguration. I believe when using r2dbc, saving data causes an error. The logs I printed indicate that it is not treating field and table names as keywords.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 18, 2025
@ShenFeng312
Copy link
Author

ShenFeng312 commented Feb 18, 2025

The cause of the bug seems to be that the default values provided by org.springframework.data.r2dbc.mapping.R2dbcMappingContext are inconsistent with JDBC. Should I submit a PR to fix this issue?

	public R2dbcMappingContext(NamingStrategy namingStrategy) {
		super(namingStrategy);
// use setForceQuote(true) to support keyword fields
		setForceQuote(false);
	}

@mp911de
Copy link
Member

mp911de commented Feb 18, 2025

We initially didn't want to enable forceQuote for R2DBC to retain compatibility. It would make sense to align for the next major revision, so feel free to submit a pull request. I assume that updating all tests will be the major effort here.

@mp911de mp911de changed the title MappingRelationalConverter cannot convert reserved keyword fields Use force-quoting in R2dbcMappingContext by default Feb 18, 2025
@mp911de mp911de added status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Feb 18, 2025
@ShenFeng312
Copy link
Author

ShenFeng312 commented Feb 26, 2025

@mp911de @schauder After changing the default values and modifying some test cases, I found that besides the changes to the default values, places like DefaultStatementMapper, UpdateMapper, and QueryMapper are all using SqlIdentifier.unquoted(xxx) by default. This might mean that more changes are required to fully meet our expectations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants