dan-s1 commented on code in PR #11783:
URL: https://github.com/apache/nifi/pull/11783#discussion_r4231762275
##########
nifi-extension-bundles/nifi-poi-bundle/nifi-poi-services/src/main/java/org/apache/nifi/excel/ExcelStartingRowSchemaInference.java:
##########
@@ -50,17 +54,27 @@ public class ExcelStartingRowSchemaInference implements
SchemaInferenceEngine<Ro
private final int firstRow;
private final CellFieldTypeReader cellFieldTypeReader;
private final DataFormatter dataFormatter;
+ private final boolean ignoreCellsBeyondHeader;
+ private final ComponentLog logger;
public ExcelStartingRowSchemaInference(RowEvaluationStrategy
rowEvaluationStrategy, int firstRow, TimeValueInference timeValueInference) {
+ this(rowEvaluationStrategy, firstRow, timeValueInference, false, null);
+ }
+
+ public ExcelStartingRowSchemaInference(RowEvaluationStrategy
rowEvaluationStrategy, int firstRow, TimeValueInference timeValueInference,
+ boolean ignoreCellsBeyondHeader,
ComponentLog logger) {
this.rowEvaluationStrategy = rowEvaluationStrategy;
this.firstRow = firstRow;
this.cellFieldTypeReader = new
StandardCellFieldTypeReader(timeValueInference);
this.dataFormatter = new DataFormatter();
+ this.ignoreCellsBeyondHeader = ignoreCellsBeyondHeader;
+ this.logger = logger;
}
Review Comment:
Since all the instances of this class use the constructor collapse to one
constructor and then you would not need to check later in the code whether the
logger is null or not.
```suggestion
public ExcelStartingRowSchemaInference(RowEvaluationStrategy
rowEvaluationStrategy, int firstRow, TimeValueInference timeValueInference,
boolean ignoreCellsBeyondHeader,
ComponentLog logger) {
this.rowEvaluationStrategy = rowEvaluationStrategy;
this.firstRow = firstRow;
this.cellFieldTypeReader = new
StandardCellFieldTypeReader(timeValueInference);
this.dataFormatter = new DataFormatter();
this.ignoreCellsBeyondHeader = ignoreCellsBeyondHeader;
this.logger = logger;
}
```
##########
nifi-extension-bundles/nifi-poi-bundle/nifi-poi-services/src/main/java/org/apache/nifi/excel/ExcelReader.java:
##########
@@ -108,6 +108,17 @@ public class ExcelReader extends SchemaRegistryService
implements RecordReaderFa
.addValidator(StandardValidators.NON_BLANK_VALIDATOR)
.build();
+ public static final PropertyDescriptor IGNORE_CELLS_BEYOND_HEADER = new
PropertyDescriptor
+ .Builder().name("Ignore Cells Beyond Header")
+ .description("Whether to ignore cells located beyond the last
column of the header row, instead of failing schema inference."
+ + " Such cells are often blank but formatted. A warning is
logged when ignored cells contain a value."
+ + " Only the rows evaluated by the '" +
ROW_EVALUATION_STRATEGY.getName() + "' property are checked.")
+ .required(true)
+ .allowableValues("true", "false")
+ .defaultValue("false")
+ .dependsOn(SchemaAccessUtils.SCHEMA_ACCESS_STRATEGY,
ExcelStartingRowSchemaInference.USE_STARTING_ROW)
+ .build();
Review Comment:
Replace with text block
```suggestion
.description("""
Whether to ignore cells located beyond the last column of
the header row, instead of failing schema inference. \
Such cells are often blank but formatted. A warning is
logged when ignored cells contain a value. \
Only the rows evaluated by the '%s' property are checked.\
""".formatted(ROW_EVALUATION_STRATEGY.getName()))
.required(true)
.allowableValues("true", "false")
.defaultValue("false")
.dependsOn(SchemaAccessUtils.SCHEMA_ACCESS_STRATEGY,
ExcelStartingRowSchemaInference.USE_STARTING_ROW)
.build();
```
##########
nifi-extension-bundles/nifi-poi-bundle/nifi-poi-services/src/main/java/org/apache/nifi/excel/ExcelStartingRowSchemaInference.java:
##########
@@ -79,16 +93,24 @@ public RecordSchema inferSchema(RecordSource<Row>
recordSource) throws IOExcepti
} else {
if (RowEvaluationStrategy.STANDARD == rowEvaluationStrategy) {
if (index <=
RowEvaluationStrategy.NUM_ROWS_TO_DETERMINE_TYPES) {
- inferSchema(row, fieldNames, typeMap);
+ inferSchema(row, fieldNames, typeMap,
ignoredCellsBySheet);
} else {
break;
}
} else {
- inferSchema(row, fieldNames, typeMap);
+ inferSchema(row, fieldNames, typeMap, ignoredCellsBySheet);
}
}
index++;
}
+
+ if (logger != null) {
+ for (Map.Entry<String, IgnoredCells> entry :
ignoredCellsBySheet.entrySet()) {
+ final IgnoredCells ignoredCells = entry.getValue();
+ logger.warn("Ignored non-empty cells beyond the {} header
columns in {} row(s) of sheet [{}], first at row {} column {}",
+ fieldNames.size(), ignoredCells.rowCount(),
entry.getKey(), ignoredCells.firstRowNumber(), ignoredCells.firstColumnName());
+ }
+ }
Review Comment:
Per earlier suggestion `logger` should never be null
```suggestion
for (Map.Entry<String, IgnoredCells> entry :
ignoredCellsBySheet.entrySet()) {
final IgnoredCells ignoredCells = entry.getValue();
logger.warn("Ignored non-empty cells beyond the {} header
columns in {} row(s) of sheet [{}], first at row {} column {}",
fieldNames.size(), ignoredCells.rowCount(), entry.getKey(),
ignoredCells.firstRowNumber(), ignoredCells.firstColumnName());
}
```
--
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]
