matrixone icon indicating copy to clipboard operation
matrixone copied to clipboard

Refactor partition state 1

Open triump2020 opened this issue 1 year ago • 1 comments

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #15530

What this PR does / why we need it:

  1. refactor partition state to optimize memory usage.

triump2020 avatar May 18 '24 11:05 triump2020

@triump2020 Thanks for your contributions!

Here are review comments for file pkg/vm/engine/disttae/logtailreplay/rows_iter.go:

Pull Request Review:

Title: Refactor partition state 1

Body:

  • The PR is categorized as an Improvement.
  • It references issue #15530.
  • The purpose of the PR is to refactor the partition state to optimize memory usage.

Changes in pkg/vm/engine/disttae/logtailreplay/rows_iter.go:

  1. Addition of NewRowsIter Function:

    • A new function NewRowsIter has been added to create a new iterator for data rows and tombstones. This function is intended to improve the handling of iterators in the codebase.
  2. Code Modification in Next Function:

    • An additional comment //FIXME::?? has been added without clear context or explanation.
    • The logic in the Next function seems to be related to skipping rows with the same RowID as the last one encountered.

Suggestions and Feedback:

  1. Comment Clarity:

    • The comment //FIXME::?? lacks clarity and does not provide sufficient information about the purpose or intention of the code. It should be revised to explain the reason for the conditional check.
  2. Code Optimization:

    • Consider providing a more descriptive comment for the logic in the Next function to enhance code readability and maintainability.
  3. Security Concerns:

    • Ensure that the refactoring does not introduce any security vulnerabilities, especially when handling sensitive data like tombstones.
  4. Testing:

    • It would be beneficial to include relevant tests to validate the changes made in the NewRowsIter function and the modified logic in the Next function.
  5. Code Consistency:

    • Ensure that the refactored code aligns with the existing code style and conventions to maintain consistency across the codebase.
  6. Documentation:

    • Update any relevant documentation to reflect the changes introduced by this refactoring.
  7. Collaboration:

    • Collaborate with team members to review the refactored code and gather feedback to ensure the changes align with the overall project goals.

By addressing the mentioned points, the pull request can be enhanced in terms of clarity, security, and maintainability.

matrix-meow avatar May 18 '24 11:05 matrix-meow

对于普通表没有问题,主要是 mo_tables, mo_columns, mo_database 这三张表有问题,因为其没有主键,rowid 也 不唯一,需要特殊处理.

triump2020 avatar May 27 '24 08:05 triump2020