tpalfy commented on code in PR #10136:
URL: https://github.com/apache/nifi/pull/10136#discussion_r2253723581
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutSQL.java:
##########
@@ -701,46 +703,64 @@ private FlowFilePoll pollFlowFiles(final ProcessContext
context, final ProcessSe
final int batchSize = context.getProperty(BATCH_SIZE).asInteger();
final FlowFileFilter dbcpServiceFlowFileFilter =
context.getProperty(CONNECTION_POOL).asControllerService(DBCPService.class).getFlowFileFilter(batchSize);
- List<FlowFile> flowFiles;
+ final List<FlowFile> validFlowFiles;
+ final List<FlowFile> selectedFlowFiles;
if (useTransactions) {
final TransactionalFlowFileFilter filter = new
TransactionalFlowFileFilter(dbcpServiceFlowFileFilter);
- flowFiles = session.get(filter);
+ selectedFlowFiles = session.get(filter);
fragmentedTransaction = filter.isFragmentedTransaction();
} else {
if (dbcpServiceFlowFileFilter == null) {
- flowFiles = session.get(batchSize);
+ selectedFlowFiles = session.get(batchSize);
} else {
- flowFiles = session.get(dbcpServiceFlowFileFilter);
+ selectedFlowFiles = session.get(dbcpServiceFlowFileFilter);
}
}
- if (flowFiles.isEmpty()) {
+ boolean selectedFlowFilesShouldHaveDataBaseNameButDont =
dbcpServiceFlowFileFilter != null
+ && !selectedFlowFiles.isEmpty()
+ &&
selectedFlowFiles.stream().findAny().get().getAttribute("database.name") ==
null;
Review Comment:
I think the objective disadvantages of such an approach is just too high.
This has more complexity with a nested if and also has 3 clauses, giving the
impression that we deal with 3 different scenarios.
Also doesn't do anything to explain the connection between the
`database.name` requirement and the type of the `FlowFileFilter`.
How about I go the classic route and inline the variable and add a comment
instead?
As for the `stream().findAny()`, the returned collection is expected to be
homogenous (whether it contains valid flowfiles - in which case all
`database.name` have the same value - or invalid flowfiles - in which case all
`database.name` are missing).
Not the same, but similar to how this is relied upon in `PutSQL` when
creating a connection:
```java
private final PartialFunctions.InitConnection<FunctionContext,
Connection> initConnection = (c, s, fc, ffs) -> {
final Connection connection =
c.getProperty(CONNECTION_POOL).asControllerService(DBCPService.class)
.getConnection(ffs == null || ffs.isEmpty() ? emptyMap() :
ffs.getFirst().getAttributes());
```
note
`ffs.getFirst().getAttributes()`
--
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]
