You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "Zouxxyy (via GitHub)" <gi...@apache.org> on 2024/09/19 05:01:17 UTC
[PR] [WIP] Introduce RowTypeProjection for ReadBuilder [paimon]
Zouxxyy opened a new pull request, #4214:
URL: https://github.com/apache/paimon/pull/4214
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #xxx
<!-- What is the purpose of the change -->
### Tests
<!-- List UT and IT cases to verify this change -->
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772475001
##########
paimon-common/src/main/java/org/apache/paimon/types/RowType.java:
##########
@@ -56,6 +56,7 @@ public final class RowType extends DataType {
private final List<DataField> fields;
private InternalRow.FieldGetter[] fieldGetters;
+ private RowType originalRowType;
Review Comment:
it should not be here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP] Introduce RowTypeProjection for ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #4214:
URL: https://github.com/apache/paimon/pull/4214#issuecomment-2360987091
We just need a `pruneColumns(RowType requiredSchema)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #4214:
URL: https://github.com/apache/paimon/pull/4214
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772474185
##########
paimon-common/src/main/java/org/apache/paimon/types/RowType.java:
##########
@@ -238,6 +275,53 @@ public RowType project(List<String> names) {
.collect(Collectors.toList()));
}
+ public RowType project(String... names) {
+ return project(Arrays.asList(names));
+ }
+
+ // These methods are just for compatibility with old code to get the projection, once
Review Comment:
If it is just for old code, please not add these methods in public `RowType`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #4214:
URL: https://github.com/apache/paimon/pull/4214#issuecomment-2370260413
@JingsongLi Thanks for review, updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772474879
##########
paimon-common/src/main/java/org/apache/paimon/types/RowType.java:
##########
@@ -238,6 +275,53 @@ public RowType project(List<String> names) {
.collect(Collectors.toList()));
}
+ public RowType project(String... names) {
+ return project(Arrays.asList(names));
+ }
+
+ // These methods are just for compatibility with old code to get the projection, once
+ // toProjection and buildWithNewProjection are not used anymore, remove them.
+ public RowType copyAndSetOriginalRowType(RowType originalRowType) {
+ if (this.originalRowType != null && !originalRowType.equals(this.originalRowType)) {
+ throw new RuntimeException();
+ }
+ return ((RowType) this.copy()).setOriginalRowType(originalRowType);
+ }
+
+ public int[] toProjection() {
+ if (fields.isEmpty()) {
+ return new int[0];
+ }
+ if (originalRowType == null) {
+ throw new RuntimeException();
+ }
+ return fields.stream()
+ .mapToInt(field -> originalRowType.getFieldIndexByFieldId(field.id()))
+ .toArray();
+ }
+
+ // create new RowType based on the new projection and originalRowType
+ public RowType buildWithNewProjection(int[] projection) {
Review Comment:
remove this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1773131596
##########
paimon-core/src/main/java/org/apache/paimon/table/source/ReadBuilder.java:
##########
@@ -111,25 +112,29 @@ default ReadBuilder withFilter(List<Predicate> predicates) {
ReadBuilder withBucketFilter(Filter<Integer> bucketFilter);
/**
- * Apply projection to the reader.
+ * Push read row type to the reader, support nested row pruning.
*
- * <p>NOTE: Nested row projection is currently not supported.
+ * @param readType read row type, can be a pruned type from {@link Table#rowType()}
+ * @since 1.0.0
*/
- default ReadBuilder withProjection(int[] projection) {
+ ReadBuilder withReadType(RowType readType);
+
+ /** Apply projection to the reader, Use {@link #withReadType(RowType)} instead. */
+ @Deprecated
+ ReadBuilder withProjection(int[] projection);
Review Comment:
I think this method can always be invalid. Not need to be Deprecated.
##########
paimon-common/src/main/java/org/apache/paimon/types/DataType.java:
##########
@@ -145,6 +145,16 @@ public boolean equals(Object o) {
return isNullable == that.isNullable && typeRoot == that.typeRoot;
}
+ /**
+ * Determine whether the current type is the result of the target type after pruning (e.g.
+ * select some fields from a nested type) or just the same.
+ *
+ * @param o the target data type
+ */
+ public boolean prunedFrom(Object o) {
Review Comment:
isPrunedFrom
##########
paimon-core/src/main/java/org/apache/paimon/io/KeyValueFileReaderFactory.java:
##########
@@ -270,141 +258,33 @@ public KeyValueFileReaderFactory build(
DeletionVector.Factory dvFactory,
boolean projectKeys,
@Nullable List<Predicate> filters) {
- int[][] keyProjection = projectKeys ? this.keyProjection : fullKeyProjection;
- RowType projectedKeyType = projectKeys ? this.projectedKeyType : keyType;
+ RowType finalReadKeyType = projectKeys ? this.readKeyType : keyType;
+ Function<TableSchema, List<DataField>> fieldsExtractor =
+ (schema) -> {
Review Comment:
(schema) -> schema
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP] Introduce RowTypeProjection for ReadBuilder [paimon]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on PR #4214:
URL: https://github.com/apache/paimon/pull/4214#issuecomment-2362593229
RowType contains all the information (field name, field id, nested structure ... ), it can replace projection
The final API will be modified to like this
```java
@Deprecated
default ReadBuilder withProjection(int[] projection) {
// projection -> requiredSchema
return pruneColumns(RowType requiredSchema);
}
ReadBuilder pruneColumns(RowType requiredSchema);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772470309
##########
paimon-common/src/main/java/org/apache/paimon/types/RowType.java:
##########
@@ -177,6 +191,25 @@ public boolean equals(Object o) {
return fields.equals(rowType.fields);
}
+ public boolean subsetOf(Object o) {
Review Comment:
override
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772473599
##########
paimon-common/src/main/java/org/apache/paimon/types/RowType.java:
##########
@@ -223,6 +256,10 @@ public RowType appendDataField(String name, DataType type) {
return new RowType(newFields);
}
+ public RowType project(int[][] mapping) {
+ return project(Arrays.stream(mapping).mapToInt(arr -> arr[0]).toArray());
Review Comment:
remove this temporary method, and just inline this to the invoker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772469691
##########
paimon-common/src/main/java/org/apache/paimon/table/SystemFields.java:
##########
@@ -42,8 +43,12 @@ public class SystemFields {
public static final DataField LEVEL =
new DataField(Integer.MAX_VALUE - 3, "_LEVEL", DataTypes.INT().notNull());
+ public static final DataField ROW_KIND =
Review Comment:
Add comment: only used by AuditLogTable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772474344
##########
paimon-common/src/main/java/org/apache/paimon/types/RowType.java:
##########
@@ -238,6 +275,53 @@ public RowType project(List<String> names) {
.collect(Collectors.toList()));
}
+ public RowType project(String... names) {
+ return project(Arrays.asList(names));
+ }
+
+ // These methods are just for compatibility with old code to get the projection, once
+ // toProjection and buildWithNewProjection are not used anymore, remove them.
+ public RowType copyAndSetOriginalRowType(RowType originalRowType) {
+ if (this.originalRowType != null && !originalRowType.equals(this.originalRowType)) {
+ throw new RuntimeException();
+ }
+ return ((RowType) this.copy()).setOriginalRowType(originalRowType);
+ }
+
+ public int[] toProjection() {
Review Comment:
it is hard to understand `RowType.toProjection`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Introduce withReadType in ReadBuilder [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4214:
URL: https://github.com/apache/paimon/pull/4214#discussion_r1772472821
##########
paimon-common/src/main/java/org/apache/paimon/types/DataType.java:
##########
@@ -145,6 +145,10 @@ public boolean equals(Object o) {
return isNullable == that.isNullable && typeRoot == that.typeRoot;
}
+ public boolean subsetOf(Object o) {
Review Comment:
add comment to explain this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org