Skip to content

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

@xinxingi xinxingi requested a review from caishunfeng as a code owner April 12, 2024 02:25
@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.

@sonarqubecloud
Copy link

@SbloodyS SbloodyS merged commit 5ad7b15 into apache:dev Apr 15, 2024
@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.

5 participants