-
Notifications
You must be signed in to change notification settings - Fork 3k
Core: Add REST catalog table requests and responses #4322
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
Core: Add REST catalog table requests and responses #4322
Conversation
|
This is blocked by #4320. Will rebase when that one is committed. |
9701a0d to
f863fa3
Compare
f863fa3 to
4245c4d
Compare
| update((MetadataUpdate.SetDefaultPartitionSpec) update); | ||
| } else if (update instanceof MetadataUpdate.SetDefaultSortOrder) { | ||
| update((MetadataUpdate.SetDefaultSortOrder) update); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you might want an else to throw at the end otherwise future updates (if there are any) may fall through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other updates are okay. These methods just add requirements, so things like AddSnapshot are supposed to fall through without changing anything.
|
|
||
| // For Jackson to properly fail when deserializing, this needs to be boxed. | ||
| // Otherwise, the boolean is parsed according to "loose" javascript JSON rules. | ||
| private Boolean dropped; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused about the conditions under which dropped would be false. According to the spec, 404 should be returned if the table is not found. 401/403 for auth. So how do we get a 200/202 response with a false flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, we need to remove 404 from the spec. The catalog API requires that the catalog should return false if the table does not exist and does not allow throwing NoSuchTableException. I'll open a PR to clarify the REST spec.
This adds REST request and response classes for table operations.
CreateTableRequestis used to create tablesUpdateTableRequestis used to update tables by applying a list ofMetadataUpdatechangesLoadTableResponseis used for create, load, and updateDropTableResponseis used for dropUpdateTableRequestincludes a set of assertions from the REST spec, with some additions that were needed to get tests working:AssertTableDoesNotExistis used for create table transactionsAssertTableUUIDchecks that the table UUID has not changed from the base metadata (prevents errors from concurrent drop/create)AssertLastAssignedFieldIdis used to check that no concurrent operation has assigned schema field IDs that may conflictAssertLastAssignedPartitionIdis used to check that no concurrent operation has assigned partition field IDs that may conflictUpdateTableRequestwill automatically add assertions for the requested changes.This is part of #4241.