voonhous commented on code in PR #19732:
URL: https://github.com/apache/hudi/pull/19732#discussion_r3860876658


##########
hudi-trino/src/main/java/io/trino/plugin/hudi/io/TrinoParquetFileReader.java:
##########
@@ -0,0 +1,461 @@
+/*
+ * Licensed 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 io.trino.plugin.hudi.io;
+
+import com.google.common.collect.ImmutableList;
+import io.trino.filesystem.TrinoFileSystem;
+import io.trino.filesystem.TrinoInputFile;
+import io.trino.memory.context.AggregatedMemoryContext;
+import io.trino.metastore.HiveType;
+import io.trino.parquet.Column;
+import io.trino.parquet.Field;
+import io.trino.parquet.ParquetCorruptionException;
+import io.trino.parquet.ParquetDataSource;
+import io.trino.parquet.ParquetReaderOptions;
+import io.trino.parquet.metadata.BlockMetadata;
+import io.trino.parquet.metadata.FileMetadata;
+import io.trino.parquet.metadata.ParquetMetadata;
+import io.trino.parquet.predicate.TupleDomainParquetPredicate;
+import io.trino.parquet.reader.MetadataReader;
+import io.trino.parquet.reader.ParquetReader;
+import io.trino.parquet.reader.RowGroupInfo;
+import io.trino.plugin.base.metrics.FileFormatDataSourceStats;
+import io.trino.plugin.hive.HiveColumnHandle;
+import io.trino.plugin.hive.parquet.ParquetReaderConfig;
+import io.trino.plugin.hudi.storage.HudiTrinoStorage;
+import io.trino.plugin.hudi.util.HudiAvroSerializer;
+import io.trino.spi.Page;
+import io.trino.spi.TrinoException;
+import io.trino.spi.connector.SourcePage;
+import io.trino.spi.predicate.TupleDomain;
+import io.trino.spi.type.SqlVarbinary;
+import io.trino.spi.type.Type;
+import org.apache.avro.Schema;
+import org.apache.avro.generic.IndexedRecord;
+import org.apache.hudi.common.bloom.BloomFilter;
+import org.apache.hudi.common.schema.HoodieSchema;
+import org.apache.hudi.common.util.collection.ClosableIterator;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.core.io.storage.HoodieAvroFileReader;
+import org.apache.hudi.storage.HoodieStorage;
+import org.apache.hudi.storage.StoragePath;
+import org.apache.parquet.column.ColumnDescriptor;
+import org.apache.parquet.io.MessageColumnIO;
+import org.apache.parquet.schema.MessageType;
+import org.joda.time.DateTimeZone;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Optional;
+import java.util.OptionalLong;
+import java.util.Set;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static 
io.trino.memory.context.AggregatedMemoryContext.newSimpleAggregatedMemoryContext;
+import static io.trino.parquet.ParquetTypeUtils.constructField;
+import static io.trino.parquet.ParquetTypeUtils.getColumnIO;
+import static io.trino.parquet.ParquetTypeUtils.getDescriptors;
+import static io.trino.parquet.ParquetTypeUtils.lookupColumnByName;
+import static io.trino.parquet.predicate.PredicateUtils.buildPredicate;
+import static io.trino.parquet.predicate.PredicateUtils.getFilteredRowGroups;
+import static 
io.trino.plugin.hive.parquet.ParquetPageSourceFactory.createDataSource;
+import static 
io.trino.plugin.hive.parquet.ParquetPageSourceFactory.getParquetMessageType;
+import static io.trino.plugin.hive.util.HiveTypeTranslator.toHiveType;
+import static io.trino.plugin.hudi.HudiErrorCode.HUDI_BAD_DATA;
+import static io.trino.plugin.hudi.HudiErrorCode.HUDI_CURSOR_ERROR;
+import static io.trino.plugin.hudi.HudiErrorCode.HUDI_SCHEMA_ERROR;
+import static io.trino.spi.type.BigintType.BIGINT;
+import static io.trino.spi.type.BooleanType.BOOLEAN;
+import static io.trino.spi.type.DoubleType.DOUBLE;
+import static io.trino.spi.type.IntegerType.INTEGER;
+import static io.trino.spi.type.RealType.REAL;
+import static io.trino.spi.type.VarbinaryType.VARBINARY;
+import static io.trino.spi.type.VarcharType.VARCHAR;
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Reads an LSM archived-timeline Parquet file through Trino's {@link 
ParquetReader}, turning each
+ * {@link Page} it produces into an Avro {@link IndexedRecord} with {@link 
HudiAvroSerializer}. Hudi's
+ * archived-timeline loader asks {@link HudiTrinoFileReaderFactory} for an 
Avro file reader over the
+ * history files under {@code .hoodie/timeline/history}, and this is the 
connector's answer to that
+ * request. It is not a general data-file reader: record-key, key-prefix and 
row-key lookups are
+ * unsupported, and so are the bloom filter and min/max record key lookups -- 
a timeline file carries
+ * none of the data-file footer metadata those rely on.
+ */
+public class TrinoParquetFileReader
+        extends HoodieAvroFileReader
+{
+    private static final String PARQUET_AVRO_SCHEMA_KEY = 
"parquet.avro.schema";
+    private static final DateTimeZone UTC_TIME_ZONE = DateTimeZone.UTC;
+    private static final int DOMAIN_COMPACTION_THRESHOLD = 1000;
+
+    private final StoragePath path;
+    private final HudiTrinoStorage trinoStorage;

Review Comment:
   Done in 8fcdd4b: it is a constructor local now and the field is gone.



##########
hudi-trino/src/test/java/io/trino/plugin/hudi/io/TestTrinoParquetFileReader.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * Licensed 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 io.trino.plugin.hudi.io;
+
+import com.google.common.io.Resources;
+import io.trino.filesystem.local.LocalFileSystem;
+import io.trino.plugin.hudi.storage.HudiTrinoStorage;
+import io.trino.plugin.hudi.storage.TrinoStorageConfiguration;
+import org.apache.avro.Schema;
+import org.apache.avro.generic.IndexedRecord;
+import org.apache.hudi.avro.model.HoodieLSMTimelineInstant;
+import org.apache.hudi.common.schema.HoodieSchema;
+import org.apache.hudi.common.table.timeline.HoodieArchivedTimeline;
+import org.apache.hudi.common.table.timeline.LSMTimeline;
+import org.apache.hudi.common.util.collection.ClosableIterator;
+import org.apache.hudi.storage.HoodieStorage;
+import org.apache.hudi.storage.StoragePath;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+import java.io.File;
+import java.nio.ByteBuffer;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Tests {@link TrinoParquetFileReader} against a four-instant LSM 
archived-timeline parquet file, the shape Hudi's
+ * archived-timeline loader reads through the connector. {@code 
archived_timeline.parquet} is the history file
+ * {@code 20250918121953134_20250918122001506_0.parquet} of a table-version-8 
COW table written by Hudi 1.0.2 (four
+ * commit instants, 20250918121953134 through 20250918122001506), generated 
with the create script documented in
+ * {@code hudi-testing-data/hudi_mor_archived_timeline.md}, COW variant.

Review Comment:
   Restored `hudi_cow_archived_timeline.md` next to the MOR one in 8fcdd4b 
(script only, no zip -- the smoke test only needs the MOR table) and pointed 
the javadoc at it. `archived_timeline.parquet` is byte-identical to that 
table's first history file, `20250918121953134_20250918122001506_0.parquet`, 
and the doc now says so. Dropped `hoodie.archive.max.commits` from the script 
as the MOR doc did; it is not a Hudi config.



##########
hudi-trino/src/test/java/io/trino/plugin/hudi/io/TestTrinoParquetFileReader.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * Licensed 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 io.trino.plugin.hudi.io;
+
+import com.google.common.io.Resources;
+import io.trino.filesystem.local.LocalFileSystem;
+import io.trino.plugin.hudi.storage.HudiTrinoStorage;
+import io.trino.plugin.hudi.storage.TrinoStorageConfiguration;
+import org.apache.avro.Schema;
+import org.apache.avro.generic.IndexedRecord;
+import org.apache.hudi.avro.model.HoodieLSMTimelineInstant;
+import org.apache.hudi.common.schema.HoodieSchema;
+import org.apache.hudi.common.table.timeline.HoodieArchivedTimeline;
+import org.apache.hudi.common.table.timeline.LSMTimeline;
+import org.apache.hudi.common.util.collection.ClosableIterator;
+import org.apache.hudi.storage.HoodieStorage;
+import org.apache.hudi.storage.StoragePath;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+import java.io.File;
+import java.nio.ByteBuffer;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Tests {@link TrinoParquetFileReader} against a four-instant LSM 
archived-timeline parquet file, the shape Hudi's
+ * archived-timeline loader reads through the connector. {@code 
archived_timeline.parquet} is the history file
+ * {@code 20250918121953134_20250918122001506_0.parquet} of a table-version-8 
COW table written by Hudi 1.0.2 (four
+ * commit instants, 20250918121953134 through 20250918122001506), generated 
with the create script documented in
+ * {@code hudi-testing-data/hudi_mor_archived_timeline.md}, COW variant.
+ */
+class TestTrinoParquetFileReader

Review Comment:
   Added both in 8fcdd4b: `testDrainedIteratorStaysDrained` drains via 
`forEachRemaining`, then asserts `hasNext()` stays false and `next()` throws 
`NoSuchElementException` (fails without the `exhausted` flag, since the second 
`nextPage()` throws); `testCloseReleasesOpenIterator` reads one record, calls 
`reader.close()` with the iterator open, then asserts `hasNext()` is false, 
`next()` throws, and closing the iterator and the reader again are no-ops.



-- 
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]

Reply via email to