Skip to content

Recognize LEFT, RIGHT, INNER, OUTER, and FULL as reserved words #2874

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
wants to merge 4 commits into from

Conversation

gregturn
Copy link
Contributor

No description provided.

@gregturn gregturn requested review from schauder and mp911de March 20, 2023 21:24
@gregturn gregturn linked an issue Mar 20, 2023 that may be closed by this pull request
@@ -700,7 +700,7 @@ reservedWord
| FOR
| FORMAT
| FROM
// | FULL
| FULL
Copy link
Member

Choose a reason for hiding this comment

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

What about the JPQL grammar about FULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've copied the test cases I used to expose this issue in HQL into the JPQL-based tests. Looks like "RIGHT" is okay, but we need to add support for "LEFT", "INNER", and "OUTER".

@gregturn gregturn changed the title Recognize LEFT, RIGHT, INNER, OUTER, and FULL as HQL reserved words Recognize LEFT, RIGHT, INNER, OUTER, and FULL a reserved words Mar 22, 2023
@@ -674,6 +674,121 @@ void orderByShouldWorkWithSubSelectStatements() {
+ "GROUP BY i2.field.id, i2.version)", sort)).endsWith("order by i.age desc");
}

@Test // GH-2864
void usingRightAsARelationshipNameShouldWork() {
Copy link
Member

Choose a reason for hiding this comment

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

These repetitive tests would shape up better as @ParametrizedTest

Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

We should use @ParametrizedTest instead of duplicating tests with different inputs.

RIGHT, LEFT, OUTER, INNER, and FULL are HQL tokens. This means they also need to be recognized as potential reserved words and thus possibly identifiers. This can show up if someone, for example, uses "right" as the name of a relationship in a JPA entity.

Resolves #2864.
@gregturn gregturn changed the title Recognize LEFT, RIGHT, INNER, OUTER, and FULL a reserved words Recognize LEFT, RIGHT, INNER, OUTER, and FULL as reserved words Mar 22, 2023
@gregturn
Copy link
Contributor Author

Last commit tweaks things to use @ParameterizedTest. All the other commits are due to me rebasing the branch to pick up recent changes to main.

@mp911de mp911de self-assigned this Mar 27, 2023
@mp911de mp911de added this to the 3.1 RC1 (2023.0.0) milestone Mar 28, 2023
@mp911de mp911de added type: bug A general bug in: query-parser Everything related to parsing JPQL or SQL labels Mar 28, 2023
mp911de pushed a commit that referenced this pull request Mar 28, 2023
RIGHT, LEFT, OUTER, INNER, and FULL are HQL tokens. This means they also need to be recognized as potential reserved words and thus possibly identifiers. This can show up if someone, for example, uses "right" as the name of a relationship in a JPA entity.

Resolves #2864.
Original pull request: #2874.
mp911de added a commit that referenced this pull request Mar 28, 2023
Use textblocks where possible.

See #2864.
Original pull request: #2874.
@mp911de
Copy link
Member

mp911de commented Mar 28, 2023

That's rebased, merged and polished now.

@mp911de mp911de closed this Mar 28, 2023
@mp911de mp911de deleted the issue/gh-2864 branch March 28, 2023 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: query-parser Everything related to parsing JPQL or SQL type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HQL query "where exists (subquery)" fails
2 participants