nsivabalan commented on code in PR #12310:
URL: https://github.com/apache/hudi/pull/12310#discussion_r1861291942
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1163,78 +1176,69 @@ public static HoodieData<HoodieRecord>
convertMetadataToColumnStatsRecords(Hoodi
}
}
- public static final String[] META_COLS_TO_ALWAYS_INDEX =
{COMMIT_TIME_METADATA_FIELD, RECORD_KEY_METADATA_FIELD,
PARTITION_PATH_METADATA_FIELD};
- public static final Set<String> META_COL_SET_TO_INDEX = new
HashSet<>(Arrays.asList(META_COLS_TO_ALWAYS_INDEX));
-
- public static List<String> getColumnsToIndex(HoodieTableConfig tableConfig,
- HoodieMetadataConfig
metadataConfig,
- List<String> columnNames,
- boolean overrideEnableCheck) {
- return getColumnsToIndex(tableConfig, metadataConfig,
Either.left(columnNames), overrideEnableCheck);
+ @VisibleForTesting
+ static final String[] META_COLS_TO_ALWAYS_INDEX =
{COMMIT_TIME_METADATA_FIELD, RECORD_KEY_METADATA_FIELD,
PARTITION_PATH_METADATA_FIELD};
+ private static final Set<String> META_COL_SET_TO_INDEX = new
HashSet<>(Arrays.asList(META_COLS_TO_ALWAYS_INDEX));
- }
-
public static List<String> getColumnsToIndex(HoodieTableConfig tableConfig,
HoodieMetadataConfig
metadataConfig,
List<String> columnNames) {
- return getColumnsToIndex(tableConfig, metadataConfig, columnNames, false);
+ return getColumnsToIndex(tableConfig, metadataConfig,
Either.left(columnNames), Option.empty());
Review Comment:
We could get the recordMerger from HoodieWriteConfig and fetch the record
type right?
why set it to Option.empty ?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1163,78 +1176,69 @@ public static HoodieData<HoodieRecord>
convertMetadataToColumnStatsRecords(Hoodi
}
}
- public static final String[] META_COLS_TO_ALWAYS_INDEX =
{COMMIT_TIME_METADATA_FIELD, RECORD_KEY_METADATA_FIELD,
PARTITION_PATH_METADATA_FIELD};
- public static final Set<String> META_COL_SET_TO_INDEX = new
HashSet<>(Arrays.asList(META_COLS_TO_ALWAYS_INDEX));
-
- public static List<String> getColumnsToIndex(HoodieTableConfig tableConfig,
- HoodieMetadataConfig
metadataConfig,
- List<String> columnNames,
- boolean overrideEnableCheck) {
- return getColumnsToIndex(tableConfig, metadataConfig,
Either.left(columnNames), overrideEnableCheck);
+ @VisibleForTesting
+ static final String[] META_COLS_TO_ALWAYS_INDEX =
{COMMIT_TIME_METADATA_FIELD, RECORD_KEY_METADATA_FIELD,
PARTITION_PATH_METADATA_FIELD};
+ private static final Set<String> META_COL_SET_TO_INDEX = new
HashSet<>(Arrays.asList(META_COLS_TO_ALWAYS_INDEX));
- }
-
public static List<String> getColumnsToIndex(HoodieTableConfig tableConfig,
HoodieMetadataConfig
metadataConfig,
List<String> columnNames) {
- return getColumnsToIndex(tableConfig, metadataConfig, columnNames, false);
+ return getColumnsToIndex(tableConfig, metadataConfig,
Either.left(columnNames), Option.empty());
}
public static List<String> getColumnsToIndex(HoodieTableConfig tableConfig,
HoodieMetadataConfig
metadataConfig,
Lazy<Option<Schema>>
tableSchema,
- boolean overrideEnableCheck) {
- return getColumnsToIndex(tableConfig, metadataConfig,
Either.right(tableSchema), overrideEnableCheck);
+ Option<HoodieRecordType>
recordType) {
+ return getColumnsToIndex(tableConfig, metadataConfig,
Either.right(tableSchema), recordType);
}
public static List<String> getColumnsToIndex(HoodieTableConfig tableConfig,
HoodieMetadataConfig
metadataConfig,
Lazy<Option<Schema>>
tableSchema) {
- return getColumnsToIndex(tableConfig, metadataConfig, tableSchema, false);
+ return getColumnsToIndex(tableConfig, metadataConfig,
Either.right(tableSchema), Option.empty());
Review Comment:
same here
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadataUtil.java:
##########
@@ -1458,8 +1462,12 @@ private static Comparable<?> coerceToComparable(Schema
schema, Object val) {
}
private static boolean canCompare(Schema schema, HoodieRecordType
recordType) {
+ return isColumnTypeSupported(schema, Option.of(recordType));
+ }
+
+ private static boolean isColumnTypeSupported(Schema schema,
Option<HoodieRecordType> recordType) {
Review Comment:
also, if the record type is not set, we should got for the super set.
i.e.
the unsupported types are:
if its AVRO -> MAP
if record type is not set or if its SPARK -> MAP, RECORD, ARRAY
--
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]
