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

[Fix-15787] Reuse code and solve the problem of complex SQL parsing exceptions in… #15833

Merged
merged 7 commits into from
Apr 15, 2024

Conversation

xinxingi
Copy link
Contributor

@xinxingi xinxingi commented Apr 12, 2024

… druid, corresponding to issue fix #15787

Purpose of the pull request

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@SbloodyS SbloodyS added bug Something isn't working 3.2.2 labels Apr 12, 2024
@SbloodyS SbloodyS added this to the 3.2.2 milestone Apr 12, 2024
@SbloodyS SbloodyS changed the title Reuse code and solve the problem of complex SQL parsing exceptions in… [Fix-15787] Reuse code and solve the problem of complex SQL parsing exceptions in… Apr 12, 2024
SbloodyS
SbloodyS previously approved these changes Apr 12, 2024
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

LGTM.

qingwli
qingwli previously approved these changes Apr 12, 2024
Copy link
Member

@qingwli qingwli left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

@xinxingi Please fix UT.

@xinxingi xinxingi dismissed stale reviews from qingwli and SbloodyS via 4ee44d7 April 15, 2024 06:15
@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 39.54%. Comparing base (7894ebb) to head (19e0261).

❗ Current head 19e0261 differs from pull request most recent head cd5c412. Consider uploading reports for the commit cd5c412 to get more accurate results

Files Patch % Lines
...source/oracle/param/OracleDataSourceProcessor.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #15833      +/-   ##
============================================
+ Coverage     39.47%   39.54%   +0.06%     
- Complexity     4993     5020      +27     
============================================
  Files          1347     1347              
  Lines         45641    45642       +1     
  Branches       4892     4891       -1     
============================================
+ Hits          18018    18050      +32     
+ Misses        25699    25668      -31     
  Partials       1924     1924              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

sonarcloud bot commented Apr 15, 2024

@SbloodyS SbloodyS merged commit 5ad7b15 into apache:dev Apr 15, 2024
60 checks passed
@xinxingi
Copy link
Contributor Author

@qingwli @SbloodyS The UT that reports the error is meaningful. The com.alibaba.druid.sql.parser.SQLParserUtils.splitAndRemoveComment tool class does not do special processing for Oracle, which requires overall execution of PL/SQL. As a result, a complete statement will be incorrectly split. After correcting the logic of the code, I conducted unit tests on a series of PL/SQL statements and multiple compound SQL statements, and the tests all passed. But I didn't submit the code because there were too many test samples. If necessary, I will provide a comprehensive test sample in the comment area or make a git submission. I will continue to follow up on the maintenance of this data source from now on

@qingwli
Copy link
Member

qingwli commented Apr 15, 2024

That's ok @xinxingi , This pr is merged, Thanks for your contributing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The sql task reports an error, and the sql parsing is incorrect.
4 participants