Skip to content

API: Add default value API #4732

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.

Sign up for GitHub

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

Closed
rzhang10 wants to merge 1 commit into from
Closed

API: Add default value API #4732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions api/src/main/java/org/apache/iceberg/types/PruneColumns.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,12 @@ public Type struct(Types.StructType struct, List<Type> fieldResults) {
sameTypes = false; // signal that some types were altered
if (field.isOptional()) {
selectedFields.add(
Types.NestedField.optional(field.fieldId(), field.name(), projectedType, field.doc()));
Types.NestedField.optional(field.fieldId(), field.name(), projectedType, field.doc(),
field.initialDefault(), field.writeDefault()));
} else {
selectedFields.add(
Types.NestedField.required(field.fieldId(), field.name(), projectedType, field.doc()));
Types.NestedField.required(field.fieldId(), field.name(), projectedType, field.doc(),
field.initialDefault(), field.writeDefault()));
}
}
}
Expand Down
51 changes: 42 additions & 9 deletions api/src/main/java/org/apache/iceberg/types/Types.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,43 +415,64 @@ public int hashCode() {

public static class NestedField implements Serializable {
public static NestedField optional(int id, String name, Type type) {
return new NestedField(true, id, name, type, null);
return new NestedField(true, id, name, type, null, null, null);
}

public static NestedField optional(int id, String name, Type type, String doc) {
return new NestedField(true, id, name, type, doc);
return new NestedField(true, id, name, type, doc, null, null);
}

public static NestedField optional(int id, String name, Type type, String doc,
Object initialDefault, Object writeDefault) {
return new NestedField(true, id, name, type, doc, initialDefault, writeDefault);
}

public static NestedField required(int id, String name, Type type) {
return new NestedField(false, id, name, type, null);
return new NestedField(false, id, name, type, null, null, null);
}

public static NestedField required(int id, String name, Type type, String doc) {
return new NestedField(false, id, name, type, doc);
return new NestedField(false, id, name, type, doc, null, null);
}

public static NestedField required(int id, String name, Type type, String doc,
Object initialDefault, Object writeDefault) {
Comment on lines +438 to +439
Copy link
Contributor

@wmoustafa wmoustafa May 10, 2022

Choose a reason for hiding this comment

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

Is it acceptable to always expect doc if we need to declare a default value?

Copy link
Contributor

@rdblue rdblue May 18, 2022

Choose a reason for hiding this comment

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

I don't see a way around this so it should be okay.

return new NestedField(false, id, name, type, doc, initialDefault, writeDefault);
}

public static NestedField of(int id, boolean isOptional, String name, Type type) {
return new NestedField(isOptional, id, name, type, null);
return new NestedField(isOptional, id, name, type, null, null, null);
}

public static NestedField of(int id, boolean isOptional, String name, Type type, String doc) {
return new NestedField(isOptional, id, name, type, doc);
return new NestedField(isOptional, id, name, type, doc, null, null);
}

public static NestedField of(int id, boolean isOptional, String name, Type type, String doc,
Object initialDefault, Object writeDefault) {
return new NestedField(isOptional, id, name, type, doc, initialDefault, writeDefault);
}

private final boolean isOptional;
private final int id;
private final String name;
private final Type type;
private final String doc;
private final Object initialDefault;
private final Object writeDefault;

private NestedField(boolean isOptional, int id, String name, Type type, String doc) {
private NestedField(
boolean isOptional, int id, String name, Type type, String doc,
Object initialDefault, Object writeDefault) {
Preconditions.checkNotNull(name, "Name cannot be null");
Preconditions.checkNotNull(type, "Type cannot be null");
this.isOptional = isOptional;
this.id = id;
this.name = name;
this.type = type;
this.doc = doc;
this.initialDefault = initialDefault;
this.writeDefault = writeDefault;
}

public boolean isOptional() {
Expand All @@ -462,7 +483,7 @@ public NestedField asOptional() {
if (isOptional) {
return this;
}
return new NestedField(true, id, name, type, doc);
return new NestedField(true, id, name, type, doc, initialDefault, writeDefault);
Copy link
Contributor

@wmoustafa wmoustafa May 12, 2022

Choose a reason for hiding this comment

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

Should the API always expect intialDefault and writeDefault? Can there be an option to provide initialDefault only, and set writeDefault to the same value?

Copy link
Contributor

@rdblue rdblue May 18, 2022

Choose a reason for hiding this comment

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

If possible, I'd really like to hide the complexity of both defaults as you're suggesting. Unfortunately, I don't see a good way to do that.

Copy link
Contributor

@wmoustafa wmoustafa May 18, 2022

Choose a reason for hiding this comment

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

+1, but I think we could handle that in the table API.

}

public boolean isRequired() {
Expand All @@ -473,7 +494,11 @@ public NestedField asRequired() {
if (!isOptional) {
return this;
}
return new NestedField(false, id, name, type, doc);
return new NestedField(false, id, name, type, doc, initialDefault, writeDefault);
}

public NestedField withWriteDefault(Object newWriteDefault) {
return new NestedField(isOptional, id, name, type, doc, initialDefault, newWriteDefault);
}

public int fieldId() {
Expand All @@ -492,6 +517,14 @@ public String doc() {
return doc;
}

public Object initialDefault() {
return initialDefault;
}

public Object writeDefault() {
return writeDefault;
}

@Override
public String toString() {
return String.format("%d: %s: %s %s",
Expand Down