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

Add configuration option to disable usage of JSqlParser for native queries. #3623

Closed
wants to merge 2 commits into from

Conversation

christophstrobl
Copy link
Member

This commit introduces the spring.data.jpa.query.native.parser property that allows to switch native query parsing to the default internal parser for scenarios where JSqlParser is on the classpath but should not be used by spring-data-jpa.

See: #2989

…eries.

This commit introduces the spring.data.jpa.query.native.parser property that allows to switch native query parsing to the default internal parser for scenarios where JSqlParser is on the classpath but should not be used by spring-data.
@christophstrobl
Copy link
Member Author

I do not consider the default option to be expressive enough to hint towards its behaviour, but I failed to come up with a better wording for it.

if (!StringUtils.hasText(name)) {
return AUTO;
}
return NativeQueryEnhancer.valueOf(name.toUpperCase());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about ObjectUtils.caseInsensitiveValueOf(…)?

*/
static NativeQueryEnhancer select(ClassLoader classLoader) {

if (!ClassUtils.isPresent("net.sf.jsqlparser.parser.JSqlParser", classLoader)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Graal Native, wouldn't we want to keep this check within a field?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the check would still work just not be initialized at build time which is not necessary since its done only once and forgotten afterwards.


private static final boolean jSqlParserPresent = ClassUtils.isPresent("net.sf.jsqlparser.parser.JSqlParser",
QueryEnhancerFactory.class.getClassLoader());
private static final NativeQueryEnhancer NATIVE_QUERY_ENHANCER;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With SpringProperties falling back to System Properties, wouldn't we want to keep the selection dynamic to allow changing system property values at runtime?

More with the rationale of setting the value later and not so much for flipping it.

Copy link
Member Author

@christophstrobl christophstrobl Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we could do it, I'd rather have it set in stone. Allowing to change values at runtime might lead to interesting side effects in its best case, and we'd have to do the lookup every single time.

@mp911de mp911de added the type: enhancement A general enhancement label Sep 26, 2024
@mp911de mp911de added this to the 3.2.11 (2023.1.11) milestone Sep 26, 2024
@mp911de mp911de linked an issue Sep 26, 2024 that may be closed by this pull request
mp911de pushed a commit that referenced this pull request Oct 1, 2024
…eries.

This commit introduces the spring.data.jpa.query.native.parser property that allows to switch native query parsing to the default internal parser for scenarios where JSqlParser is on the classpath but should not be used by spring-data.

Closes #2989
Original pull request: #3623
mp911de added a commit that referenced this pull request Oct 1, 2024
Use ObjectUtils instead of Enum.valueOf(…), move class presence check into field. Allow force-selection of JSQLParser.

Add more tests.

See #2989
Original pull request: #3623
mp911de pushed a commit that referenced this pull request Oct 1, 2024
…eries.

This commit introduces the spring.data.jpa.query.native.parser property that allows to switch native query parsing to the default internal parser for scenarios where JSqlParser is on the classpath but should not be used by spring-data.

Closes #2989
Original pull request: #3623
mp911de added a commit that referenced this pull request Oct 1, 2024
Use ObjectUtils instead of Enum.valueOf(…), move class presence check into field. Allow force-selection of JSQLParser.

Add more tests.

See #2989
Original pull request: #3623
mp911de pushed a commit that referenced this pull request Oct 1, 2024
…eries.

This commit introduces the spring.data.jpa.query.native.parser property that allows to switch native query parsing to the default internal parser for scenarios where JSqlParser is on the classpath but should not be used by spring-data.

Closes #2989
Original pull request: #3623
mp911de added a commit that referenced this pull request Oct 1, 2024
Use ObjectUtils instead of Enum.valueOf(…), move class presence check into field. Allow force-selection of JSQLParser.

Add more tests.

See #2989
Original pull request: #3623
@mp911de
Copy link
Member

mp911de commented Oct 1, 2024

That's merged, polished, and backported now.

@mp911de mp911de closed this Oct 1, 2024
@mp911de mp911de deleted the issue/2989-config-property branch October 1, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide an option to disable JSqlParserQueryEnhancer
2 participants