hudi-agent commented on code in PR #19903:
URL: https://github.com/apache/hudi/pull/19903#discussion_r3985664216
##########
hudi-common/src/main/java/org/apache/hudi/common/table/cdc/HoodieCDCUtils.java:
##########
@@ -84,6 +86,32 @@ public static HoodieSchema schemaBySupplementalLoggingMode(
}
}
+ /**
+ * Validates that the table schema is compatible with the CDC supplemental
logging mode.
+ *
+ * <p>DATA_BEFORE and DATA_BEFORE_AFTER embed the table schema in the CDC
record's
+ * {@code before}/{@code after} fields. This makes any VECTOR column a
nested field,
+ * which is unsupported because VECTOR schemas and readers currently require
top-level fields.
+ * OP_KEY_ONLY stores only the operation and record key, so images can be
reconstructed
+ * from the original file slices without nesting VECTOR columns in the CDC
schema.
+ *
+ * @param tableConfig table configuration containing the CDC settings
+ * @param tableSchema table schema to validate
+ * @throws HoodieNotSupportedException if CDC image logging is enabled for a
schema containing VECTOR columns
+ */
+ public static void validateCdcSchema(HoodieTableConfig tableConfig,
HoodieSchema tableSchema) {
+ if (!tableConfig.isCDCEnabled() ||
tableConfig.cdcSupplementalLoggingMode() ==
HoodieCDCSupplementalLoggingMode.OP_KEY_ONLY) {
+ return;
+ }
+ for (HoodieSchemaField field : tableSchema.getNonNullType().getFields()) {
+ if (field.schema().getNonNullType().getType() ==
HoodieSchemaType.VECTOR) {
+ throw new HoodieNotSupportedException("CDC supplemental logging mode "
+ tableConfig.cdcSupplementalLoggingMode()
+ + " is not supported for VECTOR column '" + field.name() + "'. Set
"
+ + HoodieTableConfig.CDC_SUPPLEMENTAL_LOGGING_MODE.key() +
"=OP_KEY_ONLY or disable CDC.");
+ }
Review Comment:
🤖 nit: `tableConfig.cdcSupplementalLoggingMode()` is called twice (once in
the guard, once in the exception message) — might be worth pulling it into a
local variable for clarity and to avoid the double lookup.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/source/reader/function/HoodieCdcSplitReaderFunction.java:
##########
@@ -280,12 +285,19 @@ private ClosableIterator<HoodieRecord<RowData>>
getFileSliceHoodieRecordIterator
/** Reads a CDC base file returning required-schema records. */
private ClosableIterator<RowData> getBaseFileIterator(String path) throws
IOException {
if (path.endsWith(HoodieFileFormat.LANCE.getFileExtension())) {
- return FormatUtils.getLanceRecordIterator(
- path, tableState.getRowType().getFieldNames(), fieldTypes,
tableState.getRequiredPositions(), getHadoopConf());
+ if (requiredSchema == null) {
+ requiredSchema =
HoodieSchemaCache.intern(HoodieSchema.parse(tableState.getRequiredSchema()));
+ }
+ return FormatUtils.getLanceRecordIterator(path, requiredSchema,
getHadoopConf());
+ }
+
+ if (readFieldTypes == null) {
Review Comment:
🤖 nit: the lazy-init-on-null-check pattern for
`readFieldTypes`/`vectorColumnInfo`/`requiredSchema` is duplicated almost
verbatim in `MergeOnReadInputFormat.getBaseFileIterator` — could be worth a
small shared helper to keep the two in sync as this evolves.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/table/cdc/HoodieCDCUtils.java:
##########
@@ -84,6 +86,32 @@ public static HoodieSchema schemaBySupplementalLoggingMode(
}
}
+ /**
+ * Validates that the table schema is compatible with the CDC supplemental
logging mode.
+ *
+ * <p>DATA_BEFORE and DATA_BEFORE_AFTER embed the table schema in the CDC
record's
+ * {@code before}/{@code after} fields. This makes any VECTOR column a
nested field,
+ * which is unsupported because VECTOR schemas and readers currently require
top-level fields.
+ * OP_KEY_ONLY stores only the operation and record key, so images can be
reconstructed
+ * from the original file slices without nesting VECTOR columns in the CDC
schema.
+ *
+ * @param tableConfig table configuration containing the CDC settings
+ * @param tableSchema table schema to validate
+ * @throws HoodieNotSupportedException if CDC image logging is enabled for a
schema containing VECTOR columns
+ */
+ public static void validateCdcSchema(HoodieTableConfig tableConfig,
HoodieSchema tableSchema) {
+ if (!tableConfig.isCDCEnabled() ||
tableConfig.cdcSupplementalLoggingMode() ==
HoodieCDCSupplementalLoggingMode.OP_KEY_ONLY) {
+ return;
+ }
Review Comment:
🤖 `getFields()` throws `IllegalStateException` for non-record schemas, and
this is now reached from `validateAgainstTableProperties` on every `initTable`.
Spark bootstrap (`HoodieSparkSqlWriter` with an empty df) creates the write
client with `HoodieSchema.NULL_SCHEMA` and then calls `bootstrap()` →
`initTable(UPSERT)`, so any CDC-enabled table in the default
`DATA_BEFORE_AFTER` mode would fail to bootstrap even without vector columns.
Could you guard on `tableSchema.getNonNullType().hasFields()` (or `getType() ==
RECORD`) before iterating?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
