LadyForest commented on code in PR #119:
URL: https://github.com/apache/flink-table-store/pull/119#discussion_r876867728


##########
flink-table-store-connector/src/main/java/org/apache/flink/table/store/connector/TableStore.java:
##########
@@ -175,28 +181,33 @@ private MergeEngine mergeEngine() {
         return options.get(MERGE_ENGINE);
     }
 
-    private FileStore buildFileStore() {
+    FileStore buildFileStore() {
         RowType partitionType = TypeUtils.project(type, partitionKeysIndex());
         FileStoreOptions fileStoreOptions = new FileStoreOptions(options);
+        String json = options.get(COMPACTION_SCANNED_MANIFEST);
+        PartitionedManifestMeta manifestMeta =
+                json == null ? null : PartitionedManifestMeta.fromJson(json);
         int[] trimmedPrimaryKeys = trimmedPrimaryKeysIndex();
         if (trimmedPrimaryKeys.length == 0) {
             return FileStoreImpl.createWithValueCount(
-                    fileStoreOptions.path(tableIdentifier).toString(),
-                    schema.id(),
-                    fileStoreOptions,
-                    user,
-                    partitionType,
-                    type);
+                            fileStoreOptions.path(tableIdentifier).toString(),
+                            schema.id(),
+                            fileStoreOptions,
+                            user,
+                            partitionType,
+                            type)
+                    .withPartitionedMeta(manifestMeta);

Review Comment:
   > Nit: could we just pass the manifestMeta as a constructor argument ? All 
arguments are constructor arguments except this manifestMeta, looks strange for 
me.
   
   I'm okay with both. I just thought the param list was too long, and this 
field is not required for every condition.
   
   



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