Skip to content

fix(rest): send the requested format version when creating a table - #3055

Open
xanderbailey wants to merge 3 commits into
apache:mainfrom
xanderbailey:xb/format_version
Open

fix(rest): send the requested format version when creating a table#3055
xanderbailey wants to merge 3 commits into
apache:mainfrom
xanderbailey:xb/format_version

Conversation

@xanderbailey

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

What changes are included in this PR?

TableCreation::format_version was dropped on the floor by the REST create path: the create request has no format-version field, so the value has to travel as the format-version table property, and the catalog never set it. Every table came back at whatever version the server defaulted to.

Send it from the field, and reject a caller-supplied format-version property. It is a reserved property, so TableMetadataBuilder already rejects it for local catalogs; silently overriding it here would instead turn a request for one version into a table at another.

Are these changes tested?

AI Disclosure

`TableCreation::format_version` was dropped on the floor by the REST
create path: the create request has no format-version field, so the
value has to travel as the `format-version` table property, and the
catalog never set it. Every table came back at whatever version the
server defaulted to.

Send it from the field, and reject a caller-supplied `format-version`
property. It is a reserved property, so `TableMetadataBuilder` already
rejects it for local catalogs; silently overriding it here would instead
turn a request for one version into a table at another.
Comment on lines +1139 to +1147
if properties.contains_key(TableProperties::PROPERTY_FORMAT_VERSION) {
return Err(Error::new(
ErrorKind::DataInvalid,
format!(
"Table properties should not contain reserved properties, but got: [{}]. Set `TableCreation::format_version` instead",
TableProperties::PROPERTY_FORMAT_VERSION
),
));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a break technically but I think it's a legitimate break that we can document in the upgrade docs for 0.11.0

We could also allow the property IF it matches the format_version provided?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TableCreation::format_version is ignored when creating a table using the Iceberg REST catalog interface

1 participant