vinothchandar commented on a change in pull request #3968:
URL: https://github.com/apache/hudi/pull/3968#discussion_r748183052
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTimelineArchiveLog.java
##########
@@ -329,7 +329,8 @@ private void writeToFile(Schema wrapperSchema,
List<IndexedRecord> records) thro
if (records.size() > 0) {
Map<HeaderMetadataType, String> header = new HashMap<>();
header.put(HoodieLogBlock.HeaderMetadataType.SCHEMA,
wrapperSchema.toString());
- HoodieAvroDataBlock block = new HoodieAvroDataBlock(records, header);
+ final String keyField =
table.getMetaClient().getTableConfig().getRecordKeyFieldProp();
Review comment:
what if its an older table, where this field is not present?
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordReader.java
##########
@@ -170,10 +182,17 @@ public void scan(Option<List<String>> keys) {
HoodieTimeline completedInstantsTimeline =
commitsTimeline.filterCompletedInstants();
HoodieTimeline inflightInstantsTimeline =
commitsTimeline.filterInflights();
try {
- // iterate over the paths
+
+ // If virtual keys are enabled, set the key field accordingly.
+ String keyField = HoodieRecord.RECORD_KEY_METADATA_FIELD;
+ if (virtualKeysEnabled) {
Review comment:
single line ternary expression? of all the places, this seems the most
apt.
```
final String keyField = virtualKeysEnabled ?
this.simpleKeyGenFields.get().getKey() : HoodieRecord.RECORD_KEY_METADATA_FIELD;
```
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java
##########
@@ -89,6 +89,10 @@
private static final Logger LOG =
LogManager.getLogger(HoodieBackedTableMetadataWriter.class);
+ // Virtual keys support for metadata table
+ private static final String RECORD_KEY_FIELD = "key";
+ private static final boolean POPULATE_META_FIELDS = false;
Review comment:
this is just for the `FILES` schema or is it true for any metadata
partition? probably
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordReader.java
##########
@@ -451,6 +472,10 @@ public boolean isWithOperationField() {
public abstract Builder withBufferSize(int bufferSize);
+ public Builder withPartition(String partitionName) {
Review comment:
rename
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordReader.java
##########
@@ -151,6 +155,14 @@ protected AbstractHoodieLogRecordReader(FileSystem fs,
String basePath, List<Str
this.instantRange = instantRange;
this.withOperationField = withOperationField;
this.enableFullScan = enableFullScan;
+
+ // virtual keys handling
+ if (!tableConfig.populateMetaFields()) {
+ this.virtualKeysEnabled = true;
+ this.simpleKeyGenFields = Option.of(
+ Pair.of(tableConfig.getRecordKeyFieldProp(),
tableConfig.getPartitionFieldProp()));
+ }
+ this.partitionName = partitionName;
Review comment:
partitionPath to be consistent with everything else
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordReader.java
##########
@@ -120,28 +120,32 @@
private int totalScannedLogFiles;
// Progress
private float progress = 0.0f;
+ // Partition name
+ private Option<String> partitionName;
+ // Virtual keys
+ private boolean virtualKeysEnabled = false;
Review comment:
why introduce another construct like `virtual keys` here? just stick to
using `populateMetaFields` and go about doing our thing?
--
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]
