Abacn commented on code in PR #36397:
URL: https://github.com/apache/beam/pull/36397#discussion_r2407830971
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java:
##########
@@ -1147,10 +1146,24 @@ public static TableRow tableRowFromMessage(
Object fieldValue = field.getValue();
if ((includeCdcColumns ||
!StorageApiCDC.COLUMNS.contains(fullName.toString()))
&& includeField.test(fieldName)) {
- tableRow.put(
- fieldName,
+ Object convertedValue =
jsonValueFromMessageValue(
- fieldDescriptor, fieldValue, true, includeField,
fullName.append(".").toString()));
+ fieldDescriptor, fieldValue, true, includeField,
fullName.append(".").toString());
Review Comment:
This introduced TableRow with `put` (GenericJson method) and `setF`
(TableRow) method both used. I doubt this would create a corrupted TableRow
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProto.java:
##########
@@ -1147,10 +1146,24 @@ public static TableRow tableRowFromMessage(
Object fieldValue = field.getValue();
if ((includeCdcColumns ||
!StorageApiCDC.COLUMNS.contains(fullName.toString()))
&& includeField.test(fieldName)) {
- tableRow.put(
- fieldName,
+ Object convertedValue =
jsonValueFromMessageValue(
- fieldDescriptor, fieldValue, true, includeField,
fullName.append(".").toString()));
+ fieldDescriptor, fieldValue, true, includeField,
fullName.append(".").toString());
+
+ // Use setF when field name is "f" to avoid IllegalArgumentException
with internal field
+ if ("f".equals(fieldName)) {
+ if (convertedValue instanceof List) {
+ @SuppressWarnings("unchecked")
+ List<TableCell> tableCells = (List<TableCell>) convertedValue;
+ tableRow.setF(tableCells);
Review Comment:
The constructed TableRow object is not quite right. fieldname ("f") is not
used in two branches. From spec I understand setF is meant to sets the entire
list of fields and their values at once. But here it is understood as a method
to set a field named "f" (not correct).
--
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]
