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

Fuzz test for deleting rows from tables #3761

Open
killme2008 opened this issue Apr 20, 2024 · 10 comments · May be fixed by #3867
Open

Fuzz test for deleting rows from tables #3761

killme2008 opened this issue Apr 20, 2024 · 10 comments · May be fixed by #3867
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@killme2008
Copy link
Contributor

What type of enhancement is this?

Tech debt reduction, Other

What does the enhancement do?

Looks like we missing the fuzz test for deleting rows from tables.

It's an important API that should be carefully tested.

Implementation challenges

@WenyXu Could you provide some docs or code to help developers that would be interested in implementing this feature? Thank you.

@killme2008 killme2008 added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 20, 2024
@hanxuanliang
Copy link

I am a beginner in Rust and am quite interested in greptime. Can I contribute to this?

@WenyXu
Copy link
Member

WenyXu commented Apr 21, 2024

I am a beginner in Rust and am quite interested in greptime. Can I contribute to this?

Hi @hanxuanliang, thank you for your interest. Fuzz tests could be a little tricky. The first challenge is how to validate deleted values. Do you have any ideas?

@hanxuanliang
Copy link

I have reviewed the current test-fuzz structure, and the general approach is as follows:

  1. A new fuzz_delete target needs to be added.
  2. Define a FuzzInput to serve as the input for the fuzz test.
  3. Implement a delete_expr, which defines the expressions required for the delete operation.
  4. Finally, assemble execute_delete, which completes the following sequence: create-table -> insert-rows -> delete-rows -> drop-table.

@WenyXu
Copy link
Member

WenyXu commented Apr 22, 2024

I have reviewed the current test-fuzz structure, and the general approach is as follows:

  1. A new fuzz_delete target needs to be added.
  2. Define a FuzzInput to serve as the input for the fuzz test.
  3. Implement a delete_expr, which defines the expressions required for the delete operation.
  4. Finally, assemble execute_delete, which completes the following sequence: create-table -> insert-rows -> delete-rows -> drop-table.

Cool, almost there.

  1. We can randomly invoke the compact/flushing of the target table(You can follow the instructions in this issue: feat: flush or compact table and region functions #3363).
  2. I recommended executing a SELECT SQL to ensure the row was deleted (i.g., ... delete-rows -> compact table(randomly, optional) -> flush table(randomly, optional) -> insert rows(randomly, optional) -> validate deleted value -> drop-table.).

@hanxuanliang
Copy link

I have reviewed the current test-fuzz structure, and the general approach is as follows:

  1. A new fuzz_delete target needs to be added.
  2. Define a FuzzInput to serve as the input for the fuzz test.
  3. Implement a delete_expr, which defines the expressions required for the delete operation.
  4. Finally, assemble execute_delete, which completes the following sequence: create-table -> insert-rows -> delete-rows -> drop-table.

Cool, almost there.

  1. We can randomly invoke the compact/flushing of the target table(You can follow the instructions in this issue: feat: flush or compact table and region functions #3363).
  2. I recommended executing a SELECT SQL to ensure the row was deleted (i.g., ... delete-rows -> compact table(randomly, optional) -> flush table(randomly, optional) -> insert rows(randomly, optional) -> validate deleted value -> drop-table.).

compact/flushing, this step, can I interpret it as: After the insert, because the LSM architecture requires that rows successfully inserted can only be deleted through compact + flush. So I understand the process should be: create-table -> insert-rows -> compact (must?) -> flush (must?) -> select-count-rows -> delete rows -> select-count-rows -> drop-table.

At the same time, I found that there is no whereExpr provided in selectExpr, so does it mean that deleteExpr only needs to delete the entire table's data?

@WenyXu
Copy link
Member

WenyXu commented Apr 22, 2024

compact/flushing, this step, can I interpret it as: After the insert, because the LSM architecture requires that rows successfully inserted can only be deleted through compact + flush.

Nope, We want to cover more cases.

So I understand the process should be: create-table -> insert-rows -> compact (must?) -> flush (must?) -> select-count-rows -> delete rows -> select-count-rows -> drop-table.

For compact (must?) -> flush (must?) steps can be considered as optional.

The whole picture will be like this:
create-table -> (insert-rows -> compact (optional) -> flush (optional) -> delete rows-> validate deleted)* -> drop-table.
And the (insert-rows -> compact (optional) -> flush (optional) -> delete rows-> validate deleted)* means these steps can be repeated multiple times.

At the same time, I found that there is no whereExpr provided in selectExpr, so does it mean that deleteExpr only needs to delete the entire table's data?

I prefer to implement the where clause for the SELECT first. The select count operation is still not good enough.

@hanxuanliang
Copy link

hanxuanliang commented Apr 23, 2024

During the process of reading tests-fuzz-README, I encountered a problem. The specific compilation error is in the following file. What aspects can I look into to find a solution?

test-fuzz-errors.txt

@killme2008
Copy link
Contributor Author

What about cargo clean and try again?

@hanxuanliang
Copy link

What about cargo clean and try again?

The issue still persists. My current platform is:

platform: Mac M2 Max
OS: macOS 14.4.1

@WenyXu
Copy link
Member

WenyXu commented Apr 23, 2024

What about cargo clean and try again?

The issue still persists. My current platform is:

platform: Mac M2 Max
OS: macOS 14.4.1

try this

cargo fuzz run fuzz_alter_table --fuzz-dir tests-fuzz -D -s none

@hanxuanliang hanxuanliang linked a pull request May 6, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants