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

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

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


##########
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 think the bigger issue here is we really don't want to be checking database types through out the code and having to issue different SQL statements. So even if we did not directly reference the Oracle class here, I would still be against needing to check DB type.
   
   We have a similar problem with this MSSQL PR:
   https://github.com/apache/nifi/pull/7245
   
   If there is absolutely no way to make Oracle and MSSQL confirm to the standard SQL that is already used, then I think a more holistic solution may be needed, possible ideas:
   - Consider an ORM, we can't use Hibernate because of licensing, could consider EclipseLink
   - Create our own ORM like layer for producing SQL statements where we detect the DB type once during start up and based on that we create some kind SQL statement factory based on DB type.



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