Re: [PR] NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch [nifi]
mattyb149 closed pull request #8266: NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch URL: https://github.com/apache/nifi/pull/8266 -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch [nifi]
mattyb149 commented on PR #8266: URL: https://github.com/apache/nifi/pull/8266#issuecomment-1912930704 +1 LGTM, thanks for the fix! Merging to support/nifi-1.x and main -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch [nifi]
mattyb149 commented on PR #8266:
URL: https://github.com/apache/nifi/pull/8266#issuecomment-1904858202
When running the unit tests with Java 8 (for a `support/nifi-1.x` backport),
I get an error because the `foo` entry shows up as `null` in the output of
`testLookupMissingJsonField()`:
`[{"foo":null,"unmentioned":{"foo":"original"}}]`
I checked some commits and everything looks the same, but I'm guessing there
was some change made to the record-based stuff in `main` that didn't make its
way back to the 1.x branch. Either way JSON is not guaranteed to have "ordered"
fields, so we should figure out why the `"foo": null` is in there (if it
shouldn't be) or check each field for its contents rather than the whole output
FlowFile.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch [nifi]
jrsteinebrey commented on code in PR #8266:
URL: https://github.com/apache/nifi/pull/8266#discussion_r1461196749
##
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/LookupRecord.java:
##
@@ -487,7 +487,13 @@ public Set lookup(final Record record, final
ProcessContext contex
final RecordPath recordPath = entry.getValue();
final RecordPathResult pathResult =
recordPath.evaluate(record);
-final List lookupFieldValues =
pathResult.getSelectedFields()
+final List allFieldValues =
pathResult.getSelectedFields().toList();
+if (allFieldValues.isEmpty()) {
Review Comment:
@mattyb149 I refactored the code to avoid streaming twice. Please review
again.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch [nifi]
mattyb149 commented on code in PR #8266:
URL: https://github.com/apache/nifi/pull/8266#discussion_r1458090443
##
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/LookupRecord.java:
##
@@ -487,7 +487,13 @@ public Set lookup(final Record record, final
ProcessContext contex
final RecordPath recordPath = entry.getValue();
final RecordPathResult pathResult =
recordPath.evaluate(record);
-final List lookupFieldValues =
pathResult.getSelectedFields()
+final List allFieldValues =
pathResult.getSelectedFields().toList();
+if (allFieldValues.isEmpty()) {
Review Comment:
I was able to get this "working" by just changing line 503 to `continue;`
after setting `hasUnmatchedValue = true`, removing the temp `rels` variable and
updating the debug line. That avoids having to call `toList()` and then
`stream()` later. But since it should not be an unmatched value, that's less
than ideal too.
I think the crux of the issue is that `getSelectedFields()` returns an empty
list, when it should return a field with an `Object[0]` in it. This is a result
of `WildcardIndexPath:67` which generates the empty list instead of a singleton
list with a empty array in it. We could add a check for `array.length == 0`
there and return the one-element empty array instead.
--
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: issues-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch [nifi]
mattyb149 commented on PR #8266: URL: https://github.com/apache/nifi/pull/8266#issuecomment-1899126576 Reviewing... -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch [nifi]
jrsteinebrey opened a new pull request, #8266: URL: https://github.com/apache/nifi/pull/8266 ### Issue Tracking - [X] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI-9677) issue created ### Pull Request Tracking - [X] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-0` - [X] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-0` ### Pull Request Formatting - [X] Pull Request based on current revision of the `main` branch - [X] Pull Request refers to a feature branch with one commit containing changes # Verification I added two new unit tests to verify my fix. I also manually tested the new code works as intended in a local build of NiFi editor. ### Build - [X] Build completed using `mvn clean install -P contrib-check` - [X] JDK 21 ### Licensing N/A ### Documentation N/A -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
