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

Ignore partition fields that are dropped from the current-schema #11868

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Fokko
Copy link
Contributor

@Fokko Fokko commented Dec 24, 2024

A second attempt to fix this (the first attempt in #11604 but that one was not safe).

When a field that's dropped that still part of an older partition spec, we currently get an error. See #11842

This will replace it with the UnknownType which will be read as a null. This will work fine, since when the evaluator encounters a null, it will assume that there is no value:

if (parts == null) {
// the predicate has no partition column
return Expressions.alwaysTrue();
}

I misused the UnknownType here, but I think it is better than returning an arbitrary type, like we do in the UnknownTransform:

@Override
public Type getResultType(Type type) {
// the actual result type is not known
return Types.StringType.get();
}

Fixes #4563

@@ -412,6 +413,24 @@ public String toString() {
}
}

public static class UnknownType extends PrimitiveType {
private static final UnknownType INSTANCE = new UnknownType();
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Fokko. I am working on a PR to add UnknownType support and just find that you've already done the most important part here : ).

I wonder if we also want to also update

public static void checkCompatibility(Schema schema, int formatVersion) {
// accumulate errors as a treemap to keep them in a reasonable order
Map<Integer, String> problems = Maps.newTreeMap();
// check each field's type and defaults
for (NestedField field : schema.lazyIdToField().values()) {
Integer minFormatVersion = MIN_FORMAT_VERSIONS.get(field.type().typeId());
if (minFormatVersion != null && formatVersion < minFormatVersion) {
problems.put(
field.fieldId(),
String.format(
"Invalid type for %s: %s is not supported until v%s",
schema.findColumnName(field.fieldId()), field.type(), minFormatVersion));
}

in this PR to prevent users from adding UnknownType to v2 table schema? Is it ok to do this in a separate PR and ensure both goes before the next release?

Tracking issue for UnknownType: #11732

@Fokko Fokko force-pushed the fd-fix-missing-field branch from c25ad2a to 3dff527 Compare January 25, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ALTER TABLE ... DROP COLUMN allows dropping a column used by old PartitionSpecs
2 participants