the-other-tim-brown commented on code in PR #627:
URL: https://github.com/apache/incubator-xtable/pull/627#discussion_r1926291523
##########
xtable-core/src/main/java/org/apache/xtable/delta/DeltaActionsConverter.java:
##########
@@ -115,16 +121,45 @@ static String getFullPathToFile(Snapshot snapshot, String
dataFilePath) {
*
* @param snapshot the commit snapshot
* @param addFile the add file action
- * @return the deletion vector representation (path of data file), or null
if no deletion vector
- * is present
+ * @return the deletion vector representation, or null if no deletion vector
is present
*/
- public String extractDeletionVectorFile(Snapshot snapshot, AddFile addFile) {
+ public InternalDeletionVector extractDeletionVector(Snapshot snapshot,
AddFile addFile) {
DeletionVectorDescriptor deletionVector = addFile.deletionVector();
if (deletionVector == null) {
return null;
}
String dataFilePath = addFile.path();
- return getFullPathToFile(snapshot, dataFilePath);
+ dataFilePath = getFullPathToFile(snapshot, dataFilePath);
+ Path deletionVectorFilePath =
deletionVector.absolutePath(snapshot.deltaLog().dataPath());
+
+ // TODO assumes deletion vector file. Need to handle inlined deletion
vectors
Review Comment:
Is there a way to detect that it is not a file? Can we throw some
unsupported operation exception for now if we hit that path?
##########
xtable-api/src/main/java/org/apache/xtable/model/storage/InternalDeletionVector.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.xtable.model.storage;
+
+import java.util.Iterator;
+import java.util.function.Supplier;
+
+import lombok.AccessLevel;
+import lombok.Builder;
+import lombok.Getter;
+import lombok.NonNull;
+import lombok.Value;
+import lombok.experimental.Accessors;
+
+@Builder(toBuilder = true, builderClassName = "Builder")
+@Accessors(fluent = true)
+@Value
+public class InternalDeletionVector {
+ // path (absolute with scheme) of data file to which this deletion vector
belongs
+ @NonNull String dataFilePath;
+
+ // physical path of the deletion vector file (absolute with scheme)
+ String deletionVectorFilePath;
+
+ // offset of deletion vector start in the deletion vector file
+ int offset;
+
+ // length of the deletion vector in the deletion vector file
+ int length;
+
+ // count of records deleted by this deletion vector
+ long countRecordsDeleted;
+
+ @Getter(AccessLevel.NONE)
+ Supplier<Iterator<Long>> deleteRecordSupplier;
+
+ public Iterator<Long> deleteRecordIterator() {
+ return deleteRecordSupplier.get();
+ }
+
+ public static class Builder {
+ public Builder deleteRecordSupplier(Supplier<Iterator<Long>>
recordsSupplier) {
+ this.deleteRecordSupplier = recordsSupplier;
Review Comment:
Why is this required? It seems very similar to what the Builder annotation
will provide
##########
xtable-api/src/main/java/org/apache/xtable/model/TableChange.java:
##########
@@ -18,22 +18,39 @@
package org.apache.xtable.model;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
import lombok.Builder;
import lombok.Value;
import org.apache.xtable.model.storage.DataFilesDiff;
+import org.apache.xtable.model.storage.InternalDeletionVector;
/**
* Captures the changes in a single commit/instant from the source table.
*
* @since 0.1
*/
@Value
-@Builder(toBuilder = true)
+@Builder(toBuilder = true, builderClassName = "Builder")
public class TableChange {
// Change in files at the specified instant
DataFilesDiff filesDiff;
+ // A commit can add deletion vectors when some records are deleted. New
deletion vectors can be
+ // added even if no new data files are added. However, as deletion vectors
are always associated
+ // with a data file, they are implicitly removed when a corresponding data
file is removed.
+ List<InternalDeletionVector> deletionVectorsAdded;
Review Comment:
When we read a snapshot, will there be deletionVectors present there as well?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
