You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "mattyb149 (via GitHub)" <gi...@apache.org> on 2023/09/22 03:59:22 UTC
[GitHub] [nifi] mattyb149 opened a new pull request, #7777: NIFI-11177: Add defensive code for null values for Iceberg
mattyb149 opened a new pull request, #7777:
URL: https://github.com/apache/nifi/pull/7777
# Summary
[NIFI-11177](https://issues.apache.org/jira/browse/NIFI-11177) This PR adds defensive checks to return null values when any conversions occur
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### 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
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [x] JDK 21
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mark-bathori commented on pull request #7777: NIFI-11177: Add defensive code for null values for Iceberg
Posted by "mark-bathori (via GitHub)" <gi...@apache.org>.
mark-bathori commented on PR #7777:
URL: https://github.com/apache/nifi/pull/7777#issuecomment-1733173199
I think the last commit is not needed since the original solution contains null check in every `convert` implementation. The only issue could be if a new implementation would be added without a null check but it is very unlikely.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #7777: NIFI-11177: Add defensive code for null values for Iceberg
Posted by "mattyb149 (via GitHub)" <gi...@apache.org>.
mattyb149 commented on PR #7777:
URL: https://github.com/apache/nifi/pull/7777#issuecomment-1731740362
This should work since the top-level `convert()` is for each RecordField in the Record, but shouldn't we have the null checks in the individual `convert()s` too just in case `convert()` is called directly from somewhere else? Or do we trust in having the Record/Map/Array converters call the new method (I'm calling it `convertWithNullCheck()`)?
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mattyb149 commented on pull request #7777: NIFI-11177: Add defensive code for null values for Iceberg
Posted by "mattyb149 (via GitHub)" <gi...@apache.org>.
mattyb149 commented on PR #7777:
URL: https://github.com/apache/nifi/pull/7777#issuecomment-1731752408
I'll push a commit with the new method and change the complex types to call that, just for safety :)
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] asfgit closed pull request #7777: NIFI-11177: Add defensive code for null values for Iceberg
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #7777: NIFI-11177: Add defensive code for null values for Iceberg
URL: https://github.com/apache/nifi/pull/7777
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] mark-bathori commented on pull request #7777: NIFI-11177: Add defensive code for null values for Iceberg
Posted by "mark-bathori (via GitHub)" <gi...@apache.org>.
mark-bathori commented on PR #7777:
URL: https://github.com/apache/nifi/pull/7777#issuecomment-1732620850
I think the last commit is not needed since the original solution contains null check in every `convert` implementation, so we have 2 options here:
1. Add null check in every `convert` method implementation, so the `convertWithNullCheck` is not needed.
2. Use the `convertWithNullCheck`, make the original `convert` method private and remove the null checks from the implementations.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org