You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/10/08 13:01:21 UTC
[GitHub] [iceberg] pvary opened a new pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
pvary opened a new pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250
2nd implementation of #2935.
Based on #3248 I was able to implement the ORC delete writers.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725782531
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ // TODO: keep full metrics for position delete file columns
Review comment:
I think it's the correct time to remove this comment now since we had merged this PR: https://github.com/apache/iceberg/pull/2942/files
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725757723
##########
File path: data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java
##########
@@ -131,8 +133,6 @@ public void testDataWriter() throws IOException {
@Test
public void testEqualityDeleteWriter() throws IOException {
- Assume.assumeFalse("ORC delete files are not supported", fileFormat == FileFormat.ORC);
-
Review comment:
Thanks for the work, Making all the tests works fine without the ORC exclusion does definitely proof the ORC write path works as expected !
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#issuecomment-941003776
Thanks @pvary for the work, I plan to check this again once we got the dependency PR #3248 merged !
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#issuecomment-943344282
@openinx: Rebased, and rerun the flaky test (TestFlinkTableSink#testReplacePartitions). Now we have a clean run.
Could you please review?
Thanks,
Peter
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730533400
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
Sorry, I think I should describe this more clear. I mean we may need to add an unit test to address the case to ensure it will use the `Record` PositionDeleteStructWriter to write the `<path, pos>` if we don't provide any `rowSchema`. That prevents us introducing new changes that will lead to write path performance regression because of the conversion from `CharSequence` to `RowData` or `InternalRow`.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726127663
##########
File path: data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java
##########
@@ -131,8 +133,6 @@ public void testDataWriter() throws IOException {
@Test
public void testEqualityDeleteWriter() throws IOException {
- Assume.assumeFalse("ORC delete files are not supported", fileFormat == FileFormat.ORC);
-
Review comment:
Thanks 😄
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730536626
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriters.java
##########
@@ -138,6 +146,15 @@ private GenericOrcWriters() {
return new MapWriter<>(key, value);
}
+ public static <T> OrcRowWriter<PositionDelete<T>> positionDelete(OrcRowWriter<T> writer,
+ Function<CharSequence, ?> pathTransformFunc) {
+ return new PositionDeleteStructWriter(writer, pathTransformFunc);
Review comment:
Nit: Pls use the `new PositionDeleteStructWriter<>(writer, pathTransformFunc)` to address the IDEA warning: `Raw use of parameterized class PositionDeleteStructWriter`
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#issuecomment-945646349
Thanks for the review and the merge @openinx!
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730855830
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
Created #3305
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725784359
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ // TODO: keep full metrics for position delete file columns
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
Review comment:
Pls file an issue to address the [comment](https://github.com/apache/iceberg/pull/2935#discussion_r723204454) from the 1st version PR, and put this issue in this project: https://github.com/apache/iceberg/projects/13#card-69425379
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725762318
##########
File path: data/src/main/java/org/apache/iceberg/data/DeleteFilter.java
##########
@@ -227,6 +229,15 @@ protected boolean shouldKeep(T item) {
return builder.build();
case ORC:
+ ORC.ReadBuilder orcBuilder = ORC.read(input)
+ .project(deleteSchema)
Review comment:
Q: why the ORC reader don't enable the reuse container switch ? Is there any intentional purpose that I did not get before ? I see the Spark ORC Reader also don't enable the switch: https://github.com/apache/iceberg/blob/7cf96f096edeceac13916676c273954fdd435513/spark/src/main/java/org/apache/iceberg/spark/source/RowDataReader.java#L160
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725782841
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ // TODO: keep full metrics for position delete file columns
Review comment:
Maybe we will also need to enable the metrics config tests for ORC format in this PR ? https://github.com/apache/iceberg/pull/2942/files#diff-38854dc8f5baaf2ee70db47f1b3d2880046d0efe3ee6d830112d71161838a6faR160
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730535657
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java
##########
@@ -110,6 +110,17 @@ protected void configureDataWrite(ORC.DataWriteBuilder builder) {
builder.createWriterFunc(SparkOrcWriter::new);
}
+ @Override
+ protected void configureEqualityDelete(ORC.DeleteWriteBuilder builder) {
+ builder.createWriterFunc((iSchema, typDesc) -> new SparkOrcWriter(iSchema, typDesc));
Review comment:
Nit: The lambda can be replaced with a method reference: `SparkOrcWriter::new` I think.
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java
##########
@@ -110,6 +110,17 @@ protected void configureDataWrite(ORC.DataWriteBuilder builder) {
builder.createWriterFunc(SparkOrcWriter::new);
}
+ @Override
+ protected void configureEqualityDelete(ORC.DeleteWriteBuilder builder) {
+ builder.createWriterFunc((iSchema, typDesc) -> new SparkOrcWriter(iSchema, typDesc));
+ }
+
+ @Override
+ protected void configurePositionDelete(ORC.DeleteWriteBuilder builder) {
+ builder.createWriterFunc((iSchema, typDesc) -> new SparkOrcWriter(iSchema, typDesc));
Review comment:
Nit: ditto.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725771501
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,77 @@ private WriteBuilder() {
}
@Override
- @SuppressWarnings("unchecked")
public void write(Record value, VectorizedRowBatch output) {
- Preconditions.checkArgument(writer instanceof RecordWriter, "writer must be a RecordWriter.");
+ Preconditions.checkArgument(value != null, "value must not be null");
int row = output.size;
output.size += 1;
- List<OrcValueWriter<?>> writers = ((RecordWriter) writer).writers();
- for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(row, value.get(c, child.getJavaClass()), output.cols[c]);
- }
+
+ writer.rootNonNullWrite(row, value, output);
+ }
+
+ @Override
+ public List<OrcValueWriter<?>> writers() {
+ return this.writer.writers();
}
@Override
public Stream<FieldMetrics<?>> metrics() {
return writer.metrics();
}
- private static class RecordWriter implements OrcValueWriter<Record> {
+ public abstract static class StructWriter<S> implements OrcValueWriter<S> {
private final List<OrcValueWriter<?>> writers;
- RecordWriter(List<OrcValueWriter<?>> writers) {
+ protected StructWriter(List<OrcValueWriter<?>> writers) {
this.writers = writers;
}
- List<OrcValueWriter<?>> writers() {
- return writers;
+ public List<OrcValueWriter<?>> writers() {
+ return this.writers;
}
@Override
- public Class<Record> getJavaClass() {
- return Record.class;
+ public Stream<FieldMetrics<?>> metrics() {
+ return writers.stream().flatMap(OrcValueWriter::metrics);
}
@Override
- @SuppressWarnings("unchecked")
- public void nonNullWrite(int rowId, Record data, ColumnVector output) {
+ public void nonNullWrite(int rowId, S value, ColumnVector output) {
StructColumnVector cv = (StructColumnVector) output;
+ write(rowId, value, c -> cv.fields[c]);
+ }
+
+ // Special case of writing the root struct
+ public void rootNonNullWrite(int rowId, S value, VectorizedRowBatch output) {
+ write(rowId, value, c -> output.cols[c]);
+ }
+
+ private void write(int rowId, S value, Function<Integer, ColumnVector> colVectorAtFunc) {
for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(rowId, data.get(c, child.getJavaClass()), cv.fields[c]);
+ OrcValueWriter writer = writers.get(c);
+ writer.write(rowId, get(value, c), colVectorAtFunc.apply(c));
}
}
+ protected abstract Object get(S struct, int index);
+ }
+
+ private static class RecordWriter extends StructWriter<Record> {
+
+ RecordWriter(List<OrcValueWriter<?>> writers) {
+ super(writers);
+ }
+
@Override
- public Stream<FieldMetrics<?>> metrics() {
- return writers.stream().flatMap(OrcValueWriter::metrics);
+ public Class<Record> getJavaClass() {
+ return Record.class;
}
+
+ @Override
+ protected Object get(Record struct, int index) {
+ return struct.get(index, writers().get(index).getJavaClass());
+ }
+
Review comment:
Nit: extra empty line .
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730535336
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
For this two cases:
1. Write `<path, pos, row-data>` into positional files, the row-data could be flink's `RowData` or spark's `InternalRow`.
2. Write `<path, pos>` without any attached row-data into positional files.
I think we could use the same `PositionDeleteStructWriter` to write the `<path, pos, row-data>` or `<path, pos>`, the difference is what kinds of `pathTransformFunc` that we will pass to:
1. For flink's `<path, pos, RowData>`, we should pass a `path -> StringData.fromString(path.toString())`;
2. For spark's `<path, pos, InternalRow>`, we should pass a `path -> UTF8String.fromString(path.toString()`;
3. For both spark and flink's `<path, pos>`, we should pass a dummy `Function.identity()` to do nothing because we will just use the `Record` writer.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730802866
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
I tried to add a `Precondition.checkArgument` to `buildPositionWriter()`, just like have you suggested, but `DeleteReadTests.testPositionDeletes()` and `DeleteReadTests.testMixedPositionAndEqualityDeletes()` tests were failing with this:
```
Create function should be provided if we write row data
java.lang.IllegalArgumentException: Create function should be provided if we write row data
at org.apache.iceberg.relocated.com.google.common.base.Preconditions.checkArgument(Preconditions.java:142)
at org.apache.iceberg.parquet.Parquet$DeleteWriteBuilder.buildPositionWriter(Parquet.java:600)
at org.apache.iceberg.data.FileHelpers.writeDeleteFile(FileHelpers.java:59)
at org.apache.iceberg.data.DeleteReadTests.testPositionDeletes(DeleteReadTests.java:287)
[..]
```
I the tests we use `FileHelper.writeDeleteFile` with a table which provides the schema and without a `createWriterFunc`. So I am not sure if it is a test only issue, or a real use-case. Any info around 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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730747021
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
Ok. Refactored to match how it is working for Parquet.
There is one thing I am not entirely comfortable with:
- If we provide `rowSchema`, but do not provide the `createWriterFunc` then we ignore the provided rowSchema. My understanding was that the value of the `rowSchema` defines wether we write rowData to the position delete file or we just use it to store the filename and the position. It turns out this is defined by the combined values of these properties.
Wouldn't it be better to have a single `storeRows` boolean flag on the `DeleteWriteBuilder` class to define this behaviour, and make the appropriate checks when creating the writer wether every required parameter is set? I thing this would make this easier to understand for the next contributors.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725771333
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,77 @@ private WriteBuilder() {
}
@Override
- @SuppressWarnings("unchecked")
public void write(Record value, VectorizedRowBatch output) {
- Preconditions.checkArgument(writer instanceof RecordWriter, "writer must be a RecordWriter.");
+ Preconditions.checkArgument(value != null, "value must not be null");
int row = output.size;
output.size += 1;
- List<OrcValueWriter<?>> writers = ((RecordWriter) writer).writers();
- for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(row, value.get(c, child.getJavaClass()), output.cols[c]);
- }
+
+ writer.rootNonNullWrite(row, value, output);
+ }
+
+ @Override
+ public List<OrcValueWriter<?>> writers() {
+ return this.writer.writers();
}
@Override
public Stream<FieldMetrics<?>> metrics() {
return writer.metrics();
}
- private static class RecordWriter implements OrcValueWriter<Record> {
+ public abstract static class StructWriter<S> implements OrcValueWriter<S> {
Review comment:
This generic `StructWriter` doesn't belongs to the concrete `OrcRowWriter<Record>`, moving this to the writer definition class `GenericOrcWriters` looks more reasonable to me.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726100694
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ // TODO: keep full metrics for position delete file columns
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
Review comment:
Created #3272
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726092630
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,77 @@ private WriteBuilder() {
}
@Override
- @SuppressWarnings("unchecked")
public void write(Record value, VectorizedRowBatch output) {
- Preconditions.checkArgument(writer instanceof RecordWriter, "writer must be a RecordWriter.");
+ Preconditions.checkArgument(value != null, "value must not be null");
int row = output.size;
output.size += 1;
- List<OrcValueWriter<?>> writers = ((RecordWriter) writer).writers();
- for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(row, value.get(c, child.getJavaClass()), output.cols[c]);
- }
+
+ writer.rootNonNullWrite(row, value, output);
+ }
+
+ @Override
+ public List<OrcValueWriter<?>> writers() {
+ return this.writer.writers();
}
@Override
public Stream<FieldMetrics<?>> metrics() {
return writer.metrics();
}
- private static class RecordWriter implements OrcValueWriter<Record> {
+ public abstract static class StructWriter<S> implements OrcValueWriter<S> {
Review comment:
Discussed here: https://github.com/apache/iceberg/pull/3248#discussion_r725838169
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730537585
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriters.java
##########
@@ -138,6 +146,15 @@ private GenericOrcWriters() {
return new MapWriter<>(key, value);
}
+ public static <T> OrcRowWriter<PositionDelete<T>> positionDelete(OrcRowWriter<T> writer,
+ Function<CharSequence, ?> pathTransformFunc) {
+ return new PositionDeleteStructWriter(writer, pathTransformFunc);
+ }
+
+ public static <T> OrcRowWriter<PositionDelete<T>> positionDelete() {
+ return new PositionDeleteWriter();
Review comment:
Could we use share the same `PositionDeleteStructWriter` as the [comment](https://github.com/apache/iceberg/pull/3250/commits/3b5423edfe4c4cbbd13be10a2c6367d87f2845c8#r730535336) described ?
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730739842
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriters.java
##########
@@ -138,6 +146,15 @@ private GenericOrcWriters() {
return new MapWriter<>(key, value);
}
+ public static <T> OrcRowWriter<PositionDelete<T>> positionDelete(OrcRowWriter<T> writer,
+ Function<CharSequence, ?> pathTransformFunc) {
+ return new PositionDeleteStructWriter(writer, pathTransformFunc);
Review comment:
Done (My IntelliJ does not complain, but fixed it anyway)
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java
##########
@@ -110,6 +110,17 @@ protected void configureDataWrite(ORC.DataWriteBuilder builder) {
builder.createWriterFunc(SparkOrcWriter::new);
}
+ @Override
+ protected void configureEqualityDelete(ORC.DeleteWriteBuilder builder) {
+ builder.createWriterFunc((iSchema, typDesc) -> new SparkOrcWriter(iSchema, typDesc));
+ }
+
+ @Override
+ protected void configurePositionDelete(ORC.DeleteWriteBuilder builder) {
+ builder.createWriterFunc((iSchema, typDesc) -> new SparkOrcWriter(iSchema, typDesc));
Review comment:
Done
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java
##########
@@ -110,6 +110,17 @@ protected void configureDataWrite(ORC.DataWriteBuilder builder) {
builder.createWriterFunc(SparkOrcWriter::new);
}
+ @Override
+ protected void configureEqualityDelete(ORC.DeleteWriteBuilder builder) {
+ builder.createWriterFunc((iSchema, typDesc) -> new SparkOrcWriter(iSchema, typDesc));
Review comment:
Done
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730793921
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
Thanks for sharing your feeling, there are two cases we did not describe it clear in current Parquet/ORC/Avro position writer builder I think:
1. `rowSchema` is null and `createWriterFunc` is not null. In this case, the `createWriterFunc` should be meaningless because we don't need to write any extra row records into position delete files. Using the default `Record` position delete writer should be OK for me.
2. `rowSchema` is not null and `createWriteFunc` is null. In this case, I think we should throw an IllegalArgumentException because we don't know how to construct the `rowSchema` 's column writers. Adding a `Precondition.checkArgument(rowSchema!=null && createWriterFunc, "..."` should be OK, but currently we will fallback to skip to write `rowSchema` row into position delete files, which is your confusion I think.
That sounds reasonable to make it into a separate PR I think.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726093658
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,77 @@ private WriteBuilder() {
}
@Override
- @SuppressWarnings("unchecked")
public void write(Record value, VectorizedRowBatch output) {
- Preconditions.checkArgument(writer instanceof RecordWriter, "writer must be a RecordWriter.");
+ Preconditions.checkArgument(value != null, "value must not be null");
int row = output.size;
output.size += 1;
- List<OrcValueWriter<?>> writers = ((RecordWriter) writer).writers();
- for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(row, value.get(c, child.getJavaClass()), output.cols[c]);
- }
+
+ writer.rootNonNullWrite(row, value, output);
+ }
+
+ @Override
+ public List<OrcValueWriter<?>> writers() {
+ return this.writer.writers();
}
@Override
public Stream<FieldMetrics<?>> metrics() {
return writer.metrics();
}
- private static class RecordWriter implements OrcValueWriter<Record> {
+ public abstract static class StructWriter<S> implements OrcValueWriter<S> {
private final List<OrcValueWriter<?>> writers;
- RecordWriter(List<OrcValueWriter<?>> writers) {
+ protected StructWriter(List<OrcValueWriter<?>> writers) {
this.writers = writers;
}
- List<OrcValueWriter<?>> writers() {
- return writers;
+ public List<OrcValueWriter<?>> writers() {
+ return this.writers;
}
@Override
- public Class<Record> getJavaClass() {
- return Record.class;
+ public Stream<FieldMetrics<?>> metrics() {
+ return writers.stream().flatMap(OrcValueWriter::metrics);
}
@Override
- @SuppressWarnings("unchecked")
- public void nonNullWrite(int rowId, Record data, ColumnVector output) {
+ public void nonNullWrite(int rowId, S value, ColumnVector output) {
StructColumnVector cv = (StructColumnVector) output;
+ write(rowId, value, c -> cv.fields[c]);
+ }
+
+ // Special case of writing the root struct
+ public void rootNonNullWrite(int rowId, S value, VectorizedRowBatch output) {
+ write(rowId, value, c -> output.cols[c]);
+ }
+
+ private void write(int rowId, S value, Function<Integer, ColumnVector> colVectorAtFunc) {
for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(rowId, data.get(c, child.getJavaClass()), cv.fields[c]);
+ OrcValueWriter writer = writers.get(c);
+ writer.write(rowId, get(value, c), colVectorAtFunc.apply(c));
}
}
+ protected abstract Object get(S struct, int index);
+ }
+
+ private static class RecordWriter extends StructWriter<Record> {
+
+ RecordWriter(List<OrcValueWriter<?>> writers) {
+ super(writers);
+ }
+
@Override
- public Stream<FieldMetrics<?>> metrics() {
- return writers.stream().flatMap(OrcValueWriter::metrics);
+ public Class<Record> getJavaClass() {
+ return Record.class;
}
+
+ @Override
+ protected Object get(Record struct, int index) {
+ return struct.get(index, writers().get(index).getJavaClass());
+ }
+
Review comment:
Discussed here https://github.com/apache/iceberg/pull/3248#discussion_r725845536
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726091166
##########
File path: data/src/main/java/org/apache/iceberg/data/DeleteFilter.java
##########
@@ -227,6 +229,15 @@ protected boolean shouldKeep(T item) {
return builder.build();
case ORC:
+ ORC.ReadBuilder orcBuilder = ORC.read(input)
+ .project(deleteSchema)
Review comment:
I think it does it automatically by using the same batch to return the data
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725780595
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
Review comment:
It's recommended to use the `MetricsConfig#forTable` ?
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#issuecomment-942881540
@pvary we've just got the https://github.com/apache/iceberg/pull/3248 merged now, I think it's time to rebase this PR and take another round review now, thanks !
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730835792
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
Looks like it's [here](https://github.com/apache/iceberg/blob/1b920e2945d6fc48661735e884e4f1919e9cdf86/parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java#L489) where we've set the rowSchema by default, while in fact we shouldn't use the `table.schema()` as `rowSchema` when building the parquet posDeleteWriter. I will suggest to use the following to construct the PositionWriter:
```java
PositionDeleteWriter<?> writer = Parquet.writeDeletes(out)
.withSpec(table.spec());
.setAll(table.properties());
.metricsConfig(MetricsConfig.forTable(table))
.withPartition(partition)
.overwrite()
.buildPositionWriter();
```
And if people plan to use `forTable(table)` to construct the position writer, then the `Preconditions.checkArgument(rowSchema == null || createWriterFunc != null)` will remind the devs to add `createWriterFunc` or fallback to use the separate setters.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730835792
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,157 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.forTable(table));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
+
+ return new EqualityDeleteWriter<>(
+ appenderBuilder.build(), FileFormat.ORC, location, spec, partition, keyMetadata,
+ sortOrder, equalityFieldIds);
+ }
+
+ public <T> PositionDeleteWriter<T> buildPositionWriter() {
+ Preconditions.checkState(equalityFieldIds == null, "Cannot create position delete file using delete field ids");
+ Preconditions.checkArgument(spec != null, "Spec must not be null when creating position delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "position");
+
+ Schema deleteSchema = DeleteSchemaUtil.posDeleteSchema(rowSchema);
+ appenderBuilder.schema(deleteSchema);
+
+ if (createWriterFunc != null) {
+ appenderBuilder.createWriterFunc((schema, typeDescription) ->
+ GenericOrcWriters.positionDelete(createWriterFunc.apply(deleteSchema, typeDescription), pathTransformFunc));
Review comment:
Looks like it's [here](https://github.com/apache/iceberg/blob/1b920e2945d6fc48661735e884e4f1919e9cdf86/parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java#L489) where we've set the rowSchema by default, while in fact we shouldn't use the `table.schema()` as `rowSchema` when building the parquet posDeleteWriter by default. I will suggest to use the following to construct the PositionWriter:
```java
PositionDeleteWriter<?> writer = Parquet.writeDeletes(out)
.withSpec(table.spec());
.setAll(table.properties());
.metricsConfig(MetricsConfig.forTable(table))
.withPartition(partition)
.overwrite()
.buildPositionWriter();
```
And if people plan to use `forTable(table)` to construct the position writer, then the `Preconditions.checkArgument(rowSchema == null || createWriterFunc != null)` will remind the devs to add `createWriterFunc` or fallback to use the separate setters.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx merged pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx merged pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726128410
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ // TODO: keep full metrics for position delete file columns
Review comment:
Removed the comment and the Assume. The tests are green.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725784359
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
+ return this;
+ }
+
+ public DeleteWriteBuilder set(String property, String value) {
+ appenderBuilder.set(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder setAll(Map<String, String> properties) {
+ appenderBuilder.setAll(properties);
+ return this;
+ }
+
+ public DeleteWriteBuilder meta(String property, String value) {
+ appenderBuilder.metadata(property, value);
+ return this;
+ }
+
+ public DeleteWriteBuilder overwrite() {
+ return overwrite(true);
+ }
+
+ public DeleteWriteBuilder overwrite(boolean enabled) {
+ appenderBuilder.overwrite(enabled);
+ return this;
+ }
+
+ public DeleteWriteBuilder metricsConfig(MetricsConfig newMetricsConfig) {
+ // TODO: keep full metrics for position delete file columns
+ appenderBuilder.metricsConfig(newMetricsConfig);
+ return this;
+ }
+
+ public DeleteWriteBuilder createWriterFunc(BiFunction<Schema, TypeDescription, OrcRowWriter<?>> newWriterFunc) {
+ this.createWriterFunc = newWriterFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder rowSchema(Schema newSchema) {
+ this.rowSchema = newSchema;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSpec(PartitionSpec newSpec) {
+ this.spec = newSpec;
+ return this;
+ }
+
+ public DeleteWriteBuilder withPartition(StructLike key) {
+ this.partition = key;
+ return this;
+ }
+
+ public DeleteWriteBuilder withKeyMetadata(EncryptionKeyMetadata metadata) {
+ this.keyMetadata = metadata;
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(List<Integer> fieldIds) {
+ this.equalityFieldIds = ArrayUtil.toIntArray(fieldIds);
+ return this;
+ }
+
+ public DeleteWriteBuilder equalityFieldIds(int... fieldIds) {
+ this.equalityFieldIds = fieldIds;
+ return this;
+ }
+
+ public DeleteWriteBuilder transformPaths(Function<CharSequence, ?> newPathTransformFunc) {
+ this.pathTransformFunc = newPathTransformFunc;
+ return this;
+ }
+
+ public DeleteWriteBuilder withSortOrder(SortOrder newSortOrder) {
+ this.sortOrder = newSortOrder;
+ return this;
+ }
+
+ public <T> EqualityDeleteWriter<T> buildEqualityWriter() {
+ Preconditions.checkState(rowSchema != null, "Cannot create equality delete file without a schema`");
+ Preconditions.checkState(equalityFieldIds != null, "Cannot create equality delete file without delete field ids");
+ Preconditions.checkState(createWriterFunc != null,
+ "Cannot create equality delete file unless createWriterFunc is set");
+ Preconditions.checkArgument(spec != null,
+ "Spec must not be null when creating equality delete writer");
+ Preconditions.checkArgument(spec.isUnpartitioned() || partition != null,
+ "Partition must not be null for partitioned writes");
+
+ meta("delete-type", "equality");
+ meta("delete-field-ids", IntStream.of(equalityFieldIds)
+ .mapToObj(Objects::toString)
+ .collect(Collectors.joining(", ")));
+
+ // the appender uses the row schema without extra columns
+ appenderBuilder.schema(rowSchema);
+ appenderBuilder.createWriterFunc(createWriterFunc);
Review comment:
Pls file an issue to address the [comment](https://github.com/apache/iceberg/pull/2935#discussion_r723204454) from the 1st version PR (So that we could track the whole progress easily :-) ), and put this issue in this project: https://github.com/apache/iceberg/projects/13#card-69425379
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r730739281
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriters.java
##########
@@ -138,6 +146,15 @@ private GenericOrcWriters() {
return new MapWriter<>(key, value);
}
+ public static <T> OrcRowWriter<PositionDelete<T>> positionDelete(OrcRowWriter<T> writer,
+ Function<CharSequence, ?> pathTransformFunc) {
+ return new PositionDeleteStructWriter(writer, pathTransformFunc);
+ }
+
+ public static <T> OrcRowWriter<PositionDelete<T>> positionDelete() {
+ return new PositionDeleteWriter();
Review comment:
Done
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#issuecomment-940051016
This seems like a better solution for #2935.
This PR is build on #3248.
The first commit in this PR is the squashed commits of #3248, the next 3 changes are unique for this.
If you have time, could you please review: @kbendick, @aokolnychyi, @rdblue?
@openinx: Answered your comments, and did the appropriate changes. Thanks for your time!
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726127887
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORC.java
##########
@@ -223,6 +245,158 @@ public DataWriteBuilder withSortOrder(SortOrder newSortOrder) {
}
}
+ public static DeleteWriteBuilder writeDeletes(OutputFile file) {
+ return new DeleteWriteBuilder(file);
+ }
+
+ public static class DeleteWriteBuilder {
+ private final WriteBuilder appenderBuilder;
+ private final String location;
+ private BiFunction<Schema, TypeDescription, OrcRowWriter<?>> createWriterFunc = null;
+ private Schema rowSchema = null;
+ private PartitionSpec spec = null;
+ private StructLike partition = null;
+ private EncryptionKeyMetadata keyMetadata = null;
+ private int[] equalityFieldIds = null;
+ private SortOrder sortOrder;
+ private Function<CharSequence, ?> pathTransformFunc = Function.identity();
+
+ private DeleteWriteBuilder(OutputFile file) {
+ this.appenderBuilder = write(file);
+ this.location = file.location();
+ }
+
+ public DeleteWriteBuilder forTable(Table table) {
+ rowSchema(table.schema());
+ withSpec(table.spec());
+ setAll(table.properties());
+ metricsConfig(MetricsConfig.fromProperties(table.properties()));
Review comment:
Done
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725779961
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,77 @@ private WriteBuilder() {
}
@Override
- @SuppressWarnings("unchecked")
public void write(Record value, VectorizedRowBatch output) {
- Preconditions.checkArgument(writer instanceof RecordWriter, "writer must be a RecordWriter.");
+ Preconditions.checkArgument(value != null, "value must not be null");
int row = output.size;
output.size += 1;
- List<OrcValueWriter<?>> writers = ((RecordWriter) writer).writers();
- for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(row, value.get(c, child.getJavaClass()), output.cols[c]);
- }
+
+ writer.rootNonNullWrite(row, value, output);
+ }
+
+ @Override
+ public List<OrcValueWriter<?>> writers() {
+ return this.writer.writers();
}
@Override
public Stream<FieldMetrics<?>> metrics() {
return writer.metrics();
}
- private static class RecordWriter implements OrcValueWriter<Record> {
+ public abstract static class StructWriter<S> implements OrcValueWriter<S> {
private final List<OrcValueWriter<?>> writers;
- RecordWriter(List<OrcValueWriter<?>> writers) {
+ protected StructWriter(List<OrcValueWriter<?>> writers) {
this.writers = writers;
}
- List<OrcValueWriter<?>> writers() {
- return writers;
+ public List<OrcValueWriter<?>> writers() {
+ return this.writers;
}
@Override
- public Class<Record> getJavaClass() {
- return Record.class;
+ public Stream<FieldMetrics<?>> metrics() {
+ return writers.stream().flatMap(OrcValueWriter::metrics);
}
@Override
- @SuppressWarnings("unchecked")
- public void nonNullWrite(int rowId, Record data, ColumnVector output) {
+ public void nonNullWrite(int rowId, S value, ColumnVector output) {
StructColumnVector cv = (StructColumnVector) output;
+ write(rowId, value, c -> cv.fields[c]);
+ }
+
+ // Special case of writing the root struct
+ public void rootNonNullWrite(int rowId, S value, VectorizedRowBatch output) {
Review comment:
Why do we need to expose the `rowId` in this `rootNonNullWrite` method ? The root `OrcRowWriter` always provide a write method like :
```java
public void write(RowData row, VectorizedRowBatch output)
```
So why not just hide the rowId inside the `rootNonNullWrite` like:
```java
public void write(RowData row, VectorizedRowBatch output){
Preconditions.checkArgument(row != null, "value must not be null");
writer.rootNonNullWrite(row, output);
}
```
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #3250: ORC: Implement buildEqualityWriter() and buildPositionWriter() - 2nd version
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r726094238
##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,77 @@ private WriteBuilder() {
}
@Override
- @SuppressWarnings("unchecked")
public void write(Record value, VectorizedRowBatch output) {
- Preconditions.checkArgument(writer instanceof RecordWriter, "writer must be a RecordWriter.");
+ Preconditions.checkArgument(value != null, "value must not be null");
int row = output.size;
output.size += 1;
- List<OrcValueWriter<?>> writers = ((RecordWriter) writer).writers();
- for (int c = 0; c < writers.size(); ++c) {
- OrcValueWriter child = writers.get(c);
- child.write(row, value.get(c, child.getJavaClass()), output.cols[c]);
- }
+
+ writer.rootNonNullWrite(row, value, output);
+ }
+
+ @Override
+ public List<OrcValueWriter<?>> writers() {
+ return this.writer.writers();
}
@Override
public Stream<FieldMetrics<?>> metrics() {
return writer.metrics();
}
- private static class RecordWriter implements OrcValueWriter<Record> {
+ public abstract static class StructWriter<S> implements OrcValueWriter<S> {
private final List<OrcValueWriter<?>> writers;
- RecordWriter(List<OrcValueWriter<?>> writers) {
+ protected StructWriter(List<OrcValueWriter<?>> writers) {
this.writers = writers;
}
- List<OrcValueWriter<?>> writers() {
- return writers;
+ public List<OrcValueWriter<?>> writers() {
+ return this.writers;
}
@Override
- public Class<Record> getJavaClass() {
- return Record.class;
+ public Stream<FieldMetrics<?>> metrics() {
+ return writers.stream().flatMap(OrcValueWriter::metrics);
}
@Override
- @SuppressWarnings("unchecked")
- public void nonNullWrite(int rowId, Record data, ColumnVector output) {
+ public void nonNullWrite(int rowId, S value, ColumnVector output) {
StructColumnVector cv = (StructColumnVector) output;
+ write(rowId, value, c -> cv.fields[c]);
+ }
+
+ // Special case of writing the root struct
+ public void rootNonNullWrite(int rowId, S value, VectorizedRowBatch output) {
Review comment:
Discussed here: https://github.com/apache/iceberg/pull/3248#discussion_r725838797
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org