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

[master] Bug 573094: TRIM function generates incorrect SQL for CriteriaBuilder (Oracle platform fix) #1129

Merged
merged 1 commit into from May 13, 2021

Conversation

rfelcman
Copy link
Contributor

This is fix related with changes in #1085 "Bug 573094: TRIM function generates incorrect SQL for CriteriaBuilder" based on bug #1084 "Bug 573094: TRIM function generates incorrect SQL for CriteriaBuilder".
It's cover Oracle DB (OraclePlatform). In Oracle DB LTRIM and RTRIM are simple SQL functions with two arguments see
LTRIM https://docs.oracle.com/database/121/SQLRF/functions108.htm#SQLRF00664
RTRIM https://docs.oracle.com/database/121/SQLRF/functions174.htm#SQLRF06104

Before this fix there were test failures in
CORE: org.eclipse.persistence.testing.tests.expressions.ReadAllExpressionTest.RightTrim
JPA: org.eclipse.persistence.testing.tests.jpa.relationships.ExpressionJUnitTestSuite.testLeftTrimWithTrimChar(ExpressionJUnitTestSuite.java:100)
org.eclipse.persistence.testing.tests.jpa.relationships.ExpressionJUnitTestSuite.testRightTrimWithTrimChar(ExpressionJUnitTestSuite.java:205)

Signed-off-by: Radek Felcman radek.felcman@oracle.com

… (Oracle platform fix)

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
@rfelcman rfelcman requested review from lukasj and dazey3 May 11, 2021 10:39
Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

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

LGTM

@rfelcman rfelcman merged commit d91883a into eclipse-ee4j:master May 13, 2021
@rfelcman rfelcman deleted the bug_jpa_573094_trim_function branch May 13, 2021 07:20
rfelcman added a commit to rfelcman/eclipselink that referenced this pull request May 13, 2021
… (Oracle platform fix) (eclipse-ee4j#1129)

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
(cherry picked from commit d91883a)
rfelcman added a commit to rfelcman/eclipselink that referenced this pull request May 13, 2021
… (Oracle platform fix) (eclipse-ee4j#1129)

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
(cherry picked from commit d91883a)
rfelcman added a commit that referenced this pull request May 13, 2021
… (Oracle platform fix) (#1129) (#1134)

This is fix related with changes in #1085 "Bug 573094: TRIM function generates incorrect SQL for CriteriaBuilder" based on bug #1084 "Bug 573094: TRIM function generates incorrect SQL for CriteriaBuilder".
It's cover Oracle DB (OraclePlatform). In Oracle DB `LTRIM` and `RTRIM` are simple SQL functions with two arguments see
LTRIM   https://docs.oracle.com/database/121/SQLRF/functions108.htm#SQLRF00664
RTRIM  https://docs.oracle.com/database/121/SQLRF/functions174.htm#SQLRF06104

Before this fix there were test failures in
CORE: `org.eclipse.persistence.testing.tests.expressions.ReadAllExpressionTest.RightTrim`
JPA: `org.eclipse.persistence.testing.tests.jpa.relationships.ExpressionJUnitTestSuite.testLeftTrimWithTrimChar(ExpressionJUnitTestSuite.java:100)`
`org.eclipse.persistence.testing.tests.jpa.relationships.ExpressionJUnitTestSuite.testRightTrimWithTrimChar(ExpressionJUnitTestSuite.java:205)`

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
(cherry picked from commit d91883a)
rfelcman added a commit that referenced this pull request May 17, 2021
… (Oracle platform fix) (#1129) (#1135)

This is fix related with changes in #1085 "Bug 573094: TRIM function generates incorrect SQL for CriteriaBuilder" based on bug #1084 "Bug 573094: TRIM function generates incorrect SQL for CriteriaBuilder".
It's cover Oracle DB (OraclePlatform). In Oracle DB LTRIM and RTRIM are simple SQL functions with two arguments see
LTRIM https://docs.oracle.com/database/121/SQLRF/functions108.htm#SQLRF00664
RTRIM https://docs.oracle.com/database/121/SQLRF/functions174.htm#SQLRF06104

Before this fix there were test failures in
CORE: org.eclipse.persistence.testing.tests.expressions.ReadAllExpressionTest.RightTrim
JPA: org.eclipse.persistence.testing.tests.jpa.relationships.ExpressionJUnitTestSuite.testLeftTrimWithTrimChar(ExpressionJUnitTestSuite.java:100)
org.eclipse.persistence.testing.tests.jpa.relationships.ExpressionJUnitTestSuite.testRightTrimWithTrimChar(ExpressionJUnitTestSuite.java:205)

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
(cherry picked from commit d91883a)
@dazey3
Copy link
Contributor

dazey3 commented May 17, 2021

@rfelcman @lukasj Thank you so much for finding this. I reviewed the documentation and I realize that I made a big mistake in my original understanding/reading of the various platforms and their definitions for LTRIM/RTRIM. My original testing for #1085 is actually wrong:

if(platform.isMySQL() || platform.isDB2() || platform.isDerby() || platform.isSymfoware()) {
    Assert.assertEquals("SELECT STRVAL1 FROM TRIMENTITY WHERE (STRVAL1 = TRIM(LEADING 'A' FROM 'AAHELLO WORDAAAAA'))", _sql.remove(0));
} else {
    Assert.assertEquals("SELECT STRVAL1 FROM TRIMENTITY WHERE (STRVAL1 = LTRIM('A', 'AAHELLO WORDAAAAA'))", _sql.remove(0));
}

For TRIM(LEADING char FROM string) functions, the order and Assert is correct, but I am incorrect for LTRIM(string, char) functions. The order is described in the documentation:

    LTRIM(char [, set])
    
    LTRIM removes from the left end of char all of the characters contained in set.

I had quickly read that (along with other similar documentation) as the first argument being the character TO BE removed from the set; the inverse.

The original reported issue of using the string argument twice for CriteriaBuilder is still valid and will be fixed by this fix. But inverting the arguments for LTRIM/RTRIM usages was something I saw during testing and also fixed as part of this issue. I will need to undo that in a new PR.

lukasj added a commit to lukasj/eclipselink that referenced this pull request Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants