[GitHub] [nifi] dam4rus commented on a diff in pull request #7338: NIFI-11631 Add OracleDB support for Nifi Registry

["dam4rus (via GitHub)" <gi...@apache.org>]

dam4rus commented on code in PR #7338:
URL: https://github.com/apache/nifi/pull/7338#discussion_r1229023824


##########
nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/DatabaseMetadataService.java:
##########
@@ -391,8 +391,12 @@ public FlowSnapshotEntity getFlowSnapshot(final String flowIdentifier, final Int
 
     @Override
     public FlowSnapshotEntity getLatestSnapshot(final String flowIdentifier) {
-        final String sql = "SELECT * FROM FLOW_SNAPSHOT WHERE flow_id = ? ORDER BY version DESC LIMIT 1";
-
+        final String sql;
+        if (jdbcTemplate.getDataSource() instanceof OracleCommonDataSource) {

Review Comment:
   I was pretty sure this is not the right way to do it and I have seen throughout the code that Flyway's `DatabaseTypeRegister.getDatabaseTypeForConnection(connection)` was used to get the database type. However, opening then closing the connection just to determine the database type, then have `jdbcTemplate` open another connection to execute the query have not felt right either. I guess it's not that big of an issue, especially if the data source is a connection pool, and go with the Flyway approach?



-- 
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