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 SQL rewrite issue #2086

Merged
merged 1 commit into from
Mar 26, 2019
Merged

fix SQL rewrite issue #2086

merged 1 commit into from
Mar 26, 2019

Conversation

scuwuyu
Copy link

@scuwuyu scuwuyu commented Mar 25, 2019

Fixes #ISSUSE_ID.

Changes proposed in this pull request:
-I reported an issue 4 months ago,#1348.
-Loxp fixed it,#1499.
-But it's not perfect,it has another issue. If the SQL is single routing and the pagesize is bigger than one,then the query result will return all rows,bacause the parameters has been rewrited in ParsingSQLRouter.route().

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7306

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 53.313%

Totals Coverage Status
Change from base Build 7305: 0.0%
Covered Lines: 8359
Relevant Lines: 15679

💛 - Coveralls

@terrymanu terrymanu requested a review from tuohai666 March 25, 2019 12:22
Copy link
Member

@tuohai666 tuohai666 left a comment

Choose a reason for hiding this comment

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

Have you tested for this issue in 4.0.0-RC1(the dev branch)?
I think I have resolved this problem via #1722, #1759, #1672, #1643.
Can you double check to findout what I fixed is the same thing as you met?

@tuohai666
Copy link
Member

Accept. The #2062 is also caused by this rewrite issue.

@tuohai666 tuohai666 merged commit d2a56c5 into apache:dev Mar 26, 2019
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.

3 participants