nsivabalan commented on code in PR #11153:
URL: https://github.com/apache/hudi/pull/11153#discussion_r1591139940


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergedReadHandle.java:
##########
@@ -52,23 +59,55 @@
 public class HoodieMergedReadHandle<T, I, K, O> extends HoodieReadHandle<T, I, 
K, O> {
 
   protected final Schema readerSchema;
+  private final Option<FileSlice> fileSliceOpt;
+  private final HoodieTableMetaClient metaClient;
+  private final TaskContextSupplier taskContextSupplier;
 
   public HoodieMergedReadHandle(HoodieWriteConfig config,
                                 Option<String> instantTime,
                                 HoodieTable<T, I, K, O> hoodieTable,
                                 Pair<String, String> partitionPathFileIDPair) {
+    this(config, instantTime, hoodieTable, hoodieTable.getMetaClient(), 
hoodieTable.getTaskContextSupplier(),
+        partitionPathFileIDPair, Option.empty());
+  }
+
+  public HoodieMergedReadHandle(HoodieWriteConfig config,
+                                Option<String> instantTime,
+                                HoodieTable<T, I, K, O> hoodieTable,
+                                HoodieTableMetaClient metaClient,
+                                TaskContextSupplier taskContextSupplier,
+                                Pair<String, String> partitionPathFileIDPair,
+                                Option<FileSlice> fileSliceOption) {
     super(config, instantTime, hoodieTable, partitionPathFileIDPair);
     readerSchema = HoodieAvroUtils.addMetadataFields(new 
Schema.Parser().parse(config.getSchema()), 
config.allowOperationMetadataField());
+    if (hoodieTable != null) {
+      this.metaClient = hoodieTable.getMetaClient();
+    } else {
+      this.metaClient = metaClient;
+    }
+    if (this.storage == null) {
+      this.storage = this.metaClient.getStorage();
+      this.fs = (FileSystem) this.storage.getFileSystem();
+    }
+    this.taskContextSupplier = taskContextSupplier;
+    fileSliceOpt = fileSliceOption.isPresent() ? fileSliceOption : 
getLatestFileSlice();
+  }
+
+  @Override
+  public HoodieStorage getStorage() {
+    // constructor in  HoodieIOHandle calls this. We do have two different 
code paths, where either of HoodieTable is null or meta client.
+    // In case metaClient not being null, we can set fileSystem after the 
constructor of HoodieIOHandle is called. Hence return null in the interim.

Review Comment:
   HoodieIOHandle has HoodieTable as an arg in its constructor. 
   On the read side, we don't really need entire hoodietable. we just need meta 
client, table config, etc. but HoodieIOHandle is common for both read and 
writes and hence I could not do much. 
   if you have good suggestions, lmk. 
   creating a phoney hoodietable on the read side is also contrived. bcoz, we 
typically create hoodietable objects from within WriteClient. 
   
   here we are 2 to 3 layers down. 
   entire HoodieBackedTableMetadataWriter does not have hoodietable instance. 
So, need to wire in from there which I felt as unnecessary. 
   
   I will continue to think of ways to make this clean. in the mean time, if 
you get any good ideas, lmk. 



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