Re: [PR] NIFI-12530: Support CREATE TABLE in Oracle database adapters [nifi]
asfgit closed pull request #8175: NIFI-12530: Support CREATE TABLE in Oracle database adapters URL: https://github.com/apache/nifi/pull/8175 -- 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-12530: Support CREATE TABLE in Oracle database adapters [nifi]
gresockj commented on PR #8175: URL: https://github.com/apache/nifi/pull/8175#issuecomment-1888304578 Thanks for the reviews -- I will merge this! -- 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-12530: Support CREATE TABLE in Oracle database adapters [nifi]
mattyb149 commented on PR #8175: URL: https://github.com/apache/nifi/pull/8175#issuecomment-1866377704 I didn't add the unit test to the older OracleDatabaseAdapter because it was basically copy/paste, but since the new code is too, I will add that test to it. The only reason for using the List instead of the StringBuilder was for the join(), I felt it awkward to have to use a `for` loop and add a comma if `i != 0`. If there are performance-related concerns I can change that as well. -- 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-12530: Support CREATE TABLE in Oracle database adapters [nifi]
EndzeitBegins commented on PR #8175: URL: https://github.com/apache/nifi/pull/8175#issuecomment-1865155224 Thank you for working on this @mattyb149. I just glanced over the code and have two remarks. Is there a reason, there is no test added for the `OracleDatabaseAdapter` implementation? Does it make sense to populate the columns into the `StringBuilder` directly, instead of putting them into a `List` and only then joining them into the `StringBuilder`. I was under the impression that the `StringBuilder` is especially designed for these kinds of iterative `String` creations. However, Java is not my primary language, so I might be in the wrong here. 路♂️ -- 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-12530: Support CREATE TABLE in Oracle database adapters [nifi]
mattyb149 opened a new pull request, #8175: URL: https://github.com/apache/nifi/pull/8175 # Summary [NIFI-12530](https://issues.apache.org/jira/browse/NIFI-12530) This PR adds support for CREATE TABLE (via UpdateDatabaseRecord) for the two existing Oracle database adapters. # 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-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 Please indicate the verification steps performed prior to pull request creation. ### Build - [x] 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
