[BUG] sanity check

[dentiny]

Summary

Briefly explain what this PR does.

Related Issues

Closes # or links to related issues.

Changes

Checklist

• Code builds correctly
• Tests have been added or updated
• Documentation updated if necessary
• I have reviewed my own changes

[gemini-code-assist]

Summary of Changes

Hello @dentiny, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a sanity check to ensure that a row deletion operation within a transaction stream always succeeds. This is a bug fix aimed at preventing unexpected states if the deletion fails.

Highlights

• Sanity Check: Added an assert! macro around the delete_row call in transaction_stream.rs to ensure that the row deletion operation always returns true, preventing potential silent failures.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[dentiny]

Assertion failure:

cargo test storage::mooncake_table::tests::test_streaming_begin_flush_delete_commit_end_flush -p moonlink

[dentiny]

if you print out the deletion vector, DV max size = 0

[gemini-code-assist]

Code Review

This pull request adds a sanity check to ensure that a row is not deleted twice within a transaction stream. The change uses assert! to enforce this invariant. My review focuses on the potential risks of using assert! in production code, which can lead to panics, and suggests alternatives for safer error handling and improved code readability.

[gemini-code-assist]

Using assert! will cause a panic in release builds if the condition is not met, which could crash the service. While the check at line 196 should prevent this, a panic might be too drastic for this kind of logic error.

If this check is intended for development, consider using debug_assert!, which is compiled out of release builds.

If this is a critical production check, for improved readability and debugging, you could extract the expression into a variable and add a descriptive panic message. This makes the intent clearer and helps diagnose issues if the assertion ever fails.

A non-panicking alternative would be to log a warning if a double-delete is attempted:

let was_present = ...;
if !was_present {
    tracing::warn!("Attempted to delete an already deleted row. This indicates a logic error.");
}

Suggested change

	assert!(stream_state
	.new_record_batches
	.get_mut(&batch_id)
	.unwrap()
	.deletions
	.delete_row(row_id);
	.delete_row(row_id));
	let was_present = stream_state
	.new_record_batches
	.get_mut(&batch_id)
	.unwrap()
	.deletions
	.delete_row(row_id);
	assert!(was_present, "Attempted to delete an already deleted row");