aokolnychyi commented on code in PR #11108:
URL: https://github.com/apache/iceberg/pull/11108#discussion_r1779286019
##########
core/src/main/java/org/apache/iceberg/ManifestReader.java:
##########
@@ -261,12 +262,12 @@ private CloseableIterable<ManifestEntry<F>> open(Schema
projection) {
AvroIterable<ManifestEntry<F>> reader =
Avro.read(file)
.project(ManifestEntry.wrapFileSchema(Types.StructType.of(fields)))
- .rename("manifest_entry", GenericManifestEntry.class.getName())
- .rename("partition", PartitionData.class.getName())
- .rename("r102", PartitionData.class.getName())
- .rename("data_file", content.fileClass())
- .rename("r2", content.fileClass())
- .classLoader(GenericManifestEntry.class.getClassLoader())
+ .createResolvingReader(
Review Comment:
Optional: I am not a big fan of how Spotless formats these closures. I would
consider a helper method.
##########
core/src/main/java/org/apache/iceberg/avro/InternalReader.java:
##########
@@ -0,0 +1,251 @@
+/*
+ * 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.avro;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
+import org.apache.avro.LogicalType;
+import org.apache.avro.LogicalTypes;
+import org.apache.avro.Schema;
+import org.apache.avro.io.DatumReader;
+import org.apache.avro.io.Decoder;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.Pair;
+
+/**
+ * A reader that produces Iceberg's internal in-memory object model.
+ *
+ * <p>Iceberg's internal in-memory object model produces the types defined in
{@link
+ * Type.TypeID#javaClass()}.
+ *
+ * @param <T> Java type returned by the reader
+ */
+public class InternalReader<T> implements DatumReader<T>, SupportsRowPosition {
Review Comment:
Do we need anything in the name to indicate it is specific to Avro? Will the
name clash once we add a similar path for Parquet?
##########
core/src/main/java/org/apache/iceberg/avro/InternalReader.java:
##########
@@ -0,0 +1,251 @@
+/*
+ * 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.avro;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
+import org.apache.avro.LogicalType;
+import org.apache.avro.LogicalTypes;
+import org.apache.avro.Schema;
+import org.apache.avro.io.DatumReader;
+import org.apache.avro.io.Decoder;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.Pair;
+
+/**
+ * A reader that produces Iceberg's internal in-memory object model.
+ *
+ * <p>Iceberg's internal in-memory object model produces the types defined in
{@link
+ * Type.TypeID#javaClass()}.
+ *
+ * @param <T> Java type returned by the reader
+ */
+public class InternalReader<T> implements DatumReader<T>, SupportsRowPosition {
+
+ private final Types.StructType expectedType;
+ private final Map<Integer, Class<? extends StructLike>> typeMap =
Maps.newHashMap();
+ private final Map<Integer, Object> idToConstant = ImmutableMap.of();
+ private Schema fileSchema = null;
+ private ValueReader<T> reader = null;
+
+ public static <D> InternalReader<D> create(org.apache.iceberg.Schema schema)
{
+ return new InternalReader<>(schema);
+ }
+
+ InternalReader(org.apache.iceberg.Schema readSchema) {
+ this.expectedType = readSchema.asStruct();
+ }
+
+ @SuppressWarnings("unchecked")
+ private void initReader() {
+ this.reader =
+ (ValueReader<T>)
+ AvroWithPartnerVisitor.visit(
+ Pair.of(-1, expectedType),
+ fileSchema,
+ new ResolvingReadBuilder(),
+ AccessByID.instance());
+ }
+
+ @Override
+ public void setSchema(Schema schema) {
+ this.fileSchema = schema;
+ initReader();
+ }
+
+ public InternalReader<T> setRootType(Class<? extends StructLike> rootClass) {
+ typeMap.put(-1, rootClass);
+ return this;
+ }
+
+ public InternalReader<T> setCustomType(int fieldId, Class<? extends
StructLike> structClass) {
+ typeMap.put(fieldId, structClass);
+ return this;
+ }
+
+ @Override
+ public void setRowPositionSupplier(Supplier<Long> posSupplier) {
+ if (reader instanceof SupportsRowPosition) {
+ ((SupportsRowPosition) reader).setRowPositionSupplier(posSupplier);
+ }
+ }
+
+ @Override
+ public T read(T reuse, Decoder decoder) throws IOException {
+ return reader.read(decoder, reuse);
+ }
+
+ private class ResolvingReadBuilder
+ extends AvroWithPartnerVisitor<Pair<Integer, Type>, ValueReader<?>> {
+ @Override
+ public ValueReader<?> record(
+ Pair<Integer, Type> partner, Schema record, List<ValueReader<?>>
fieldResults) {
+ if (partner == null) {
+ return ValueReaders.skipStruct(fieldResults);
+ }
+
+ Types.StructType expected = partner.second().asStructType();
+ List<Pair<Integer, ValueReader<?>>> readPlan =
+ ValueReaders.buildReadPlan(expected, record, fieldResults,
idToConstant);
+
+ return structReader(readPlan, partner.first(), expected);
+ }
+
+ private ValueReader<?> structReader(
+ List<Pair<Integer, ValueReader<?>>> readPlan, int fieldId,
Types.StructType struct) {
+
+ Class<? extends StructLike> structClass = typeMap.get(fieldId);
+ if (structClass != null) {
+ return InternalReaders.struct(struct, structClass, readPlan);
+ } else {
+ return InternalReaders.struct(struct, readPlan);
+ }
+ }
+
+ @Override
+ public ValueReader<?> union(
+ Pair<Integer, Type> partner, Schema union, List<ValueReader<?>>
options) {
+ return ValueReaders.union(options);
+ }
+
+ @Override
+ public ValueReader<?> arrayMap(
+ Pair<Integer, Type> partner,
+ Schema map,
+ ValueReader<?> keyReader,
+ ValueReader<?> valueReader) {
+ return ValueReaders.arrayMap(keyReader, valueReader);
+ }
+
+ @Override
+ public ValueReader<?> array(
+ Pair<Integer, Type> partner, Schema array, ValueReader<?>
elementReader) {
+ return ValueReaders.array(elementReader);
+ }
+
+ @Override
+ public ValueReader<?> map(Pair<Integer, Type> partner, Schema map,
ValueReader<?> valueReader) {
+ return ValueReaders.map(ValueReaders.strings(), valueReader);
+ }
+
+ @Override
+ public ValueReader<?> primitive(Pair<Integer, Type> partner, Schema
primitive) {
+ LogicalType logicalType = primitive.getLogicalType();
+ if (logicalType != null) {
+ switch (logicalType.getName()) {
+ case "date":
Review Comment:
These string constants seem fragile but I don't have a better suggestion.
##########
core/src/main/java/org/apache/iceberg/avro/InternalReader.java:
##########
@@ -0,0 +1,251 @@
+/*
+ * 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.avro;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
+import org.apache.avro.LogicalType;
+import org.apache.avro.LogicalTypes;
+import org.apache.avro.Schema;
+import org.apache.avro.io.DatumReader;
+import org.apache.avro.io.Decoder;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.Pair;
+
+/**
+ * A reader that produces Iceberg's internal in-memory object model.
+ *
+ * <p>Iceberg's internal in-memory object model produces the types defined in
{@link
+ * Type.TypeID#javaClass()}.
+ *
+ * @param <T> Java type returned by the reader
+ */
+public class InternalReader<T> implements DatumReader<T>, SupportsRowPosition {
+
+ private final Types.StructType expectedType;
+ private final Map<Integer, Class<? extends StructLike>> typeMap =
Maps.newHashMap();
+ private final Map<Integer, Object> idToConstant = ImmutableMap.of();
+ private Schema fileSchema = null;
+ private ValueReader<T> reader = null;
+
+ public static <D> InternalReader<D> create(org.apache.iceberg.Schema schema)
{
+ return new InternalReader<>(schema);
+ }
+
+ InternalReader(org.apache.iceberg.Schema readSchema) {
+ this.expectedType = readSchema.asStruct();
+ }
+
+ @SuppressWarnings("unchecked")
+ private void initReader() {
+ this.reader =
+ (ValueReader<T>)
+ AvroWithPartnerVisitor.visit(
+ Pair.of(-1, expectedType),
+ fileSchema,
+ new ResolvingReadBuilder(),
+ AccessByID.instance());
+ }
+
+ @Override
+ public void setSchema(Schema schema) {
+ this.fileSchema = schema;
+ initReader();
+ }
+
+ public InternalReader<T> setRootType(Class<? extends StructLike> rootClass) {
+ typeMap.put(-1, rootClass);
Review Comment:
What about defining a constant with a meaningful name for `-1`?
##########
core/src/main/java/org/apache/iceberg/avro/InternalReader.java:
##########
@@ -0,0 +1,251 @@
+/*
+ * 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.avro;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
+import org.apache.avro.LogicalType;
+import org.apache.avro.LogicalTypes;
+import org.apache.avro.Schema;
+import org.apache.avro.io.DatumReader;
+import org.apache.avro.io.Decoder;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.Pair;
+
+/**
+ * A reader that produces Iceberg's internal in-memory object model.
+ *
+ * <p>Iceberg's internal in-memory object model produces the types defined in
{@link
+ * Type.TypeID#javaClass()}.
+ *
+ * @param <T> Java type returned by the reader
+ */
+public class InternalReader<T> implements DatumReader<T>, SupportsRowPosition {
+
+ private final Types.StructType expectedType;
+ private final Map<Integer, Class<? extends StructLike>> typeMap =
Maps.newHashMap();
+ private final Map<Integer, Object> idToConstant = ImmutableMap.of();
Review Comment:
Do we actually need this field if it is immutable? Will it change once we
add default values?
##########
core/src/test/java/org/apache/iceberg/TestManifestReader.java:
##########
@@ -44,7 +44,8 @@ public class TestManifestReader extends TestBase {
"fileOrdinal",
"fileSequenceNumber",
"fromProjectionPos",
- "manifestLocation")
+ "manifestLocation",
+ "partitionData.partitionType.fieldsById")
Review Comment:
What did we need 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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
