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: throw error when adding fields to non-existent table in WAL #25525

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

hiltontj
Copy link
Contributor

@hiltontj hiltontj commented Nov 8, 2024

Closes #25524

In addition to throwing an error when an AddFields operation is applied for a table that does not exist, this PR adds some helper methods to the influxdb3_wal crate for creating WAL operations that were previously added in pro.

A test was added to check for the failure condition.

Copy link
Contributor

@praveen-influx praveen-influx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

) -> WalContents {
WalContents {
min_timestamp_ns,
max_timestamp_ns,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for embedding the unit ns, really helps when reading the code without full context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should do that everywhere in the code where we don't have a typed time.

@hiltontj hiltontj merged commit 3bb63b2 into main Nov 8, 2024
13 checks passed
@hiltontj hiltontj deleted the hiltontj/add-fields-error branch November 8, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CatalogOp::AddFields should throw an error if applied to non-existent table
3 participants