chenjunjiedada commented on a change in pull request #1316: URL: https://github.com/apache/iceberg/pull/1316#discussion_r468296622
########## File path: core/src/main/java/org/apache/iceberg/deletes/PositionDeleteWriter.java ########## @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.deletes; + +import java.io.Closeable; +import java.io.IOException; +import java.nio.ByteBuffer; +import org.apache.iceberg.DeleteFile; +import org.apache.iceberg.FileFormat; +import org.apache.iceberg.FileMetadata; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.StructLike; +import org.apache.iceberg.encryption.EncryptionKeyMetadata; +import org.apache.iceberg.io.FileAppender; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.types.Types; + +public class PositionDeleteWriter<T> implements Closeable { + private final FileAppender<StructLike> appender; + private final FileFormat format; + private final String location; + private final PartitionSpec spec; + private final StructLike partition; + private final ByteBuffer keyMetadata; + private final PositionDelete<T> delete; + private DeleteFile deleteFile = null; + + public PositionDeleteWriter(FileAppender<StructLike> appender, FileFormat format, String location, Review comment: Good idea to have `FileAppender` as a parameter so that engines could build their own appenders! I was thinking about how to build a writer like this. ########## File path: core/src/main/java/org/apache/iceberg/MetadataColumns.java ########## @@ -31,11 +31,20 @@ private MetadataColumns() { } + // IDs Integer.MAX_VALUE - (1-100) are used for metadata columns public static final NestedField FILE_PATH = NestedField.required( Integer.MAX_VALUE - 1, "_file", Types.StringType.get(), "Path of the file in which a row is stored"); public static final NestedField ROW_POSITION = NestedField.required( Integer.MAX_VALUE - 2, "_pos", Types.LongType.get(), "Ordinal position of a row in the source data file"); + // IDs Integer.MAX_VALUE - (101-200) are used for reserved columns + public static final NestedField DELETE_FILE_PATH = NestedField.required( Review comment: Why we need reserved columns upon metadata columns? The column docs are the same for both, will this confuse others if they want to use? ########## File path: core/src/main/java/org/apache/iceberg/MetadataColumns.java ########## @@ -31,11 +31,20 @@ private MetadataColumns() { } + // IDs Integer.MAX_VALUE - (1-100) are used for metadata columns public static final NestedField FILE_PATH = NestedField.required( Integer.MAX_VALUE - 1, "_file", Types.StringType.get(), "Path of the file in which a row is stored"); public static final NestedField ROW_POSITION = NestedField.required( Integer.MAX_VALUE - 2, "_pos", Types.LongType.get(), "Ordinal position of a row in the source data file"); + // IDs Integer.MAX_VALUE - (101-200) are used for reserved columns + public static final NestedField DELETE_FILE_PATH = NestedField.required( + Integer.MAX_VALUE - 101, "file_path", Types.StringType.get(), "Path of a file in which a deleted row is stored"); + public static final NestedField DELETE_FILE_POS = NestedField.required( + Integer.MAX_VALUE - 102, "pos", Types.LongType.get(), "Ordinal position of a deleted row in the data file"); + public static final int DELETE_FILE_ROW_FIELD_ID = Integer.MAX_VALUE - 103; Review comment: Do we need to update spec if we want to store extra row value in position delete file? ########## File path: core/src/main/java/org/apache/iceberg/deletes/PositionDelete.java ########## @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.deletes; + +import org.apache.iceberg.StructLike; + +@SuppressWarnings("checkstyle:OverloadMethodsDeclarationOrder") +public class PositionDelete<R> implements StructLike { Review comment: This should replace `PositionBasedDeleteRecord` in https://github.com/apache/iceberg/pull/971, right? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
