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

add issue 16178 cases #16545

Merged
merged 2 commits into from
Jun 1, 2024
Merged

add issue 16178 cases #16545

merged 2 commits into from
Jun 1, 2024

Conversation

heni02
Copy link
Contributor

@heni02 heni02 commented May 31, 2024

User description

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #16178

What this PR does / why we need it:

add issue 16178 cases


PR Type

Tests


Description

  • Added new test cases for issue [Bug]: Batch insert fails #16178.
  • Created storage table with columns collecttime, value, account, and interval.
  • Inserted multiple rows into storage table using prepared statements.
  • Selected and verified data from storage table to ensure correctness.

Changes walkthrough 📝

Relevant files
Tests
prepare.result
Add test case for `storage` table with prepared statements

test/distributed/cases/prepare/prepare.result

  • Added new test case for storage table creation.
  • Inserted multiple rows into storage table using prepared statements.
  • Selected and verified data from storage table.
  • +47/-0   
    prepare.test
    Add test case for `storage` table with prepared statements

    test/distributed/cases/prepare/prepare.test

  • Added new test case for storage table creation.
  • Inserted multiple rows into storage table using prepared statements.
  • Selected and verified data from storage table.
  • +40/-1   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the PR primarily involves adding new test cases and SQL statements for database operations. The changes are straightforward and localized to specific test files, making the review process relatively simple.

    🧪 Relevant tests

    Yes

    ⚡ Possible issues

    No

    🔒 Security concerns

    No

    @matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label May 31, 2024
    @mergify mergify bot requested a review from sukki37 May 31, 2024 07:02
    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add a check to ensure the table does not already exist before creating it

    Consider adding a check to ensure the table storage does not already exist before creating
    it. This will prevent potential errors if the script is run multiple times.

    test/distributed/cases/prepare/prepare.result [432-438]

    -CREATE TABLE `storage` (
    +CREATE TABLE IF NOT EXISTS `storage` (
     `collecttime` DATETIME NOT NULL,
     `value` DOUBLE NOT NULL,
     `account` VARCHAR(128) NOT NULL,
     `interval` INT NOT NULL,
     PRIMARY KEY (`collecttime`,`account`)
     );
     
    Suggestion importance[1-10]: 8

    Why: Adding a check for the existence of the table before creating it is a good practice to prevent errors in scripts that might be run multiple times. This suggestion correctly identifies a potential improvement in the SQL script to enhance robustness.

    8
    Data integrity
    Add constraints to enforce valid ranges for value and interval columns

    To ensure data integrity, consider adding constraints to the value and interval columns to
    enforce valid ranges.

    test/distributed/cases/prepare/prepare.result [434-436]

    -`value` DOUBLE NOT NULL,
    +`value` DOUBLE NOT NULL CHECK (`value` >= 0),
     `account` VARCHAR(128) NOT NULL,
    -`interval` INT NOT NULL,
    +`interval` INT NOT NULL CHECK (`interval` > 0),
     
    Suggestion importance[1-10]: 7

    Why: Adding constraints to ensure data integrity is a valuable suggestion. It enhances the reliability of the database by preventing invalid data entries. The suggestion correctly targets the new columns added in the PR and proposes a practical improvement.

    7

    @mergify mergify bot added the kind/bug Something isn't working label May 31, 2024
    @mergify mergify bot merged commit f1c1dae into matrixorigin:1.2-dev Jun 1, 2024
    17 of 18 checks passed
    @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 Review effort [1-5]: 2 size/S Denotes a PR that changes [10,99] lines Tests
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    4 participants