[Kernel] Throw InvalidTableException when we encounter a known invalid table state #3288
Conversation
| ) | ||
| } | ||
|
|
||
| // TODO address the inconsistent behaviors and throw better error messages for corrupt listings? |
There was a problem hiding this comment.
I also addressed this in this PR since it was an easy fix. See this change.
allisonport-db
changed the title
kernel/kernel-api/src/main/java/io/delta/kernel/internal/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
| "Log file not found.\nExpected: %s\nFound: %s", | ||
| FileNames.deltaFile(logPath, newCheckpointVersion + 1), | ||
| FileNames.deltaFile(logPath, deltaVersionsAfterCheckpoint.get(0)) | ||
| throw new InvalidTableException( |
There was a problem hiding this comment.
DeltaErrors.invalidTable(Path tablePath, String msg, Object args...)?
There was a problem hiding this comment.
I wasn't sure if we needed a method for this. If it just passes the args exactly the same and it's a specific exception class it seems like we can just throw the exception in line? What do you think?
| FileNames.deltaVersion(deltas.get(deltas.size() - 1).getPath()) < newVersion) { | ||
| throw new InvalidTableException( | ||
| tablePath.toString(), | ||
| String.format("Missing delta file for version %s", newVersion) |
There was a problem hiding this comment.
Missing delta log file(s) needed for state construction?
There was a problem hiding this comment.
Not sure if it matters but this error you actually can reconstruct the state... (since there is a checkpoint file) but we throw an error anyways (in most cases, see the test suite... it's not all cases) because there should be a delta file for the checkpoint version present
…hot/SnapshotManager.java Co-authored-by: Venki Korukanti <[email protected]>
Which Delta project/connector is this regarding?
Description
During log reconstruction we throw a bunch of various errors when we encounter a table state that we consider invalid. This PR updates some of those errors to be
InvalidTableException. This doesn't necessarily cover all possible exceptions due to an invalid table, just some of the known and clear cases during log reconstruction.How was this patch tested?
Updates existing tests.
Does this PR introduce any user-facing changes?
Yes, instead of internal
RuntimeExceptionsorIllegalStateExceptions, etc aInvalidTableExceptionwill be thrown when certain invalid table states are encountered.