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 Bvt case in issue 14836 in 1.2 #15966

Merged
merged 1 commit into from
May 10, 2024

Conversation

xzxiong
Copy link
Contributor

@xzxiong xzxiong commented May 10, 2024

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #14836

What this PR does / why we need it:

ref #15914
changes:

  1. move op check statement_info result to the end of bvt.

changes:
1. move op `check statement_info result` to the end of bvt.

Approved by: @heni02
@matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label May 10, 2024
@mergify mergify bot requested a review from sukki37 May 10, 2024 04:05
@mergify mergify bot added the kind/bug Something isn't working label May 10, 2024
@matrix-meow
Copy link
Contributor

@xzxiong Thanks for your contributions!

Pull Request Review:

Title:

The title of the pull request is clear and concise, indicating that it fixes a specific issue related to Bvt case in issue 14836 in version 1.2.

Body:

The body of the pull request provides information about the type of PR (BUG), the specific issue it fixes (issue #14836), and a reference to another related issue (#15914). It also mentions the changes made in the PR, which involve moving an operation related to checking statement_info result to the end of Bvt.

Changes:

  1. test/distributed/cases/log/query_version.result:

    • The changes involve moving the operation select count(1) as cnt, statement_id, statement, status from system.statement_info group by statement_id, statement, status having count(1) > 1; to the end of the file.
    • The removal of select * from user limit 0; and other unnecessary statements.
    • The addition of /* cloud_user */ before select * from user limit 0;.
  2. test/distributed/cases/log/query_version.sql:

    • The changes include adding comments related to issue fixes and moving the operation select * from user limit 0; to the end of the file.
  3. test/distributed/cases/result_count/result_count.result:

    • The changes involve replacing select sleep(16); with select sleep(18) s;.
  4. test/distributed/cases/result_count/result_count.sql:

    • Similar to the result file, the SQL file changes select sleep(16); to select sleep(18) s;.
  5. test/distributed/cases/sql_source_type/sql_source_type.result:

    • The changes involve removing unnecessary select sleep(16); statements and adding comments related to issue fixes.
  6. test/distributed/cases/sql_source_type/sql_source_type.sql:

    • Similar to the result file, the SQL file adds comments related to issue fixes and removes select sleep(16);.
  7. test/distributed/cases/zz_statement_query_type/query_version.result:

    • A new file is added with specific select statements related to system.statement_info.
  8. test/distributed/cases/zz_statement_query_type/query_version.sql:

    • A new file is added with select statements related to system.statement_info and issue fixes.
  9. test/distributed/cases/zz_statement_query_type/sql_source_type.result:

    • A new file is added with select statements related to system.statement_info.
  10. test/distributed/cases/zz_statement_query_type/sql_source_type.sql:

    • A new file is added with select statements related to system.statement_info and issue fixes.

Suggestions for Improvement:

  1. Code Optimization:

    • Consider consolidating similar changes across multiple files to reduce redundancy and improve maintainability.
    • Remove commented-out code or unnecessary statements to keep the codebase clean and easy to understand.
  2. Security Concerns:

    • Ensure that sensitive information like passwords or account details are not inadvertently exposed in the code changes.
    • Review any SQL queries to prevent SQL injection vulnerabilities by using parameterized queries or input sanitization.
  3. Documentation:

    • Add inline comments to explain the purpose of complex or critical code sections for better code comprehension by other developers.
  4. Consistency:

    • Maintain consistency in coding style, such as indentation, commenting practices, and naming conventions, throughout the codebase.
  5. Testing:

    • Verify that the changes do not introduce regressions by running relevant tests and ensuring that the functionality works as expected.
  6. Collaboration:

    • Encourage collaboration

@mergify mergify bot merged commit 22d81a3 into matrixorigin:1.2-dev May 10, 2024
19 checks passed
@xzxiong xzxiong deleted the fix_issue_14836_12 branch May 10, 2024 09:32
@aylei aylei mentioned this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working size/S Denotes a PR that changes [10,99] lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants