[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[Pierre Villard (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16449495#comment-16449495
 ] 

Pierre Villard commented on NIFI-5082:
--

[~mike.thomsen] - when merging a PR, don't forget about closing the associated 
Jira by clicking "Resolve Issue" and setting a fix version :) Thanks!

> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Extensions
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
> Fix For: 1.7.0
>
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16447423#comment-16447423
 ] 

ASF GitHub Bot commented on NIFI-5082:
--

Github user asfgit closed the pull request at:

https://github.com/apache/nifi/pull/2638


> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16447422#comment-16447422
 ] 

ASF subversion and git services commented on NIFI-5082:
---

Commit ba32879ec81cdfa3c44734ab8b210b13fa0581ac in nifi's branch 
refs/heads/master from [~ca9mbu]
[ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=ba32879 ]

NIFI-5082: Added support for custom Oracle timestamp types to Avro conversion

This closes #2638

Signed-off-by: Mike Thomsen 


> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16447421#comment-16447421
 ] 

ASF GitHub Bot commented on NIFI-5082:
--

Github user MikeThomsen commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2638#discussion_r183256566
  
--- Diff: 
nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/ResultSetRecordSet.java
 ---
@@ -350,6 +350,8 @@ private static RecordFieldType getFieldType(final int 
sqlType) {
 return RecordFieldType.TIME;
 case Types.TIMESTAMP:
 case Types.TIMESTAMP_WITH_TIMEZONE:
+case -101: // Oracle's TIMESTAMP WITH TIME ZONE
--- End diff --

@mattyb149 @markap14 Given how proprietary Oracle can be, I don't see how 
this could hurt because it's going to be difficult to say what they'll do in 
the future and how Calcite might respond to that.


> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16445673#comment-16445673
 ] 

ASF GitHub Bot commented on NIFI-5082:
--

Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2638
  
+1 LGTM. Passed the build and it matches my rough understanding of the 
issues w/ Oracle as well. I'm good to merge this if you don't think it needs 
further review.


> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16440950#comment-16440950
 ] 

ASF GitHub Bot commented on NIFI-5082:
--

Github user mattyb149 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2638#discussion_r182100896
  
--- Diff: 
nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/ResultSetRecordSet.java
 ---
@@ -350,6 +350,8 @@ private static RecordFieldType getFieldType(final int 
sqlType) {
 return RecordFieldType.TIME;
 case Types.TIMESTAMP:
 case Types.TIMESTAMP_WITH_TIMEZONE:
+case -101: // Oracle's TIMESTAMP WITH TIME ZONE
--- End diff --

@markap14 I don't think we need these cases (Calcite would never return 
Oracle types, and there'd be no other way to generate them?) but put them in 
just in case (as this does Java SQL Timestamp handling). If you agree then I 
can remove this (although it's likely no-harm-no-foul)


> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16440948#comment-16440948
 ] 

ASF GitHub Bot commented on NIFI-5082:
--

Github user mattyb149 commented on the issue:

https://github.com/apache/nifi/pull/2638
  
I REALLY didn't want to have to do this explicit Oracle stuff in the 
generic code, but the other options I tried weren't viable. I tried isolating 
the SQL type conversion to the DatabaseAdapters, but ExecuteSQL uses JdbcCommon 
and does not use DatabaseAdapters, so the problem would still exist. I also 
tried calling getTimestamp() in the default handling block (before attempting 
to store a String) but that is really brittle since other drivers may or may 
not handle getTimestamp() in a manner we could rely on. This is a simple 
straightforward (but dirty) fix.


> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16440941#comment-16440941
 ] 

ASF GitHub Bot commented on NIFI-5082:
--

GitHub user mattyb149 opened a pull request:

https://github.com/apache/nifi/pull/2638

NIFI-5082: Added support for custom Oracle timestamp types to Avro 
conversion

Thank you for submitting a contribution to Apache NiFi.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced 
 in the commit message?

- [x] Does your PR title start with NIFI- where  is the JIRA number 
you are trying to resolve? Pay particular attention to the hyphen "-" character.

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically master)?

- [x] Is your initial contribution a single, squashed commit?

### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn 
-Pcontrib-check clean install at the root nifi folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
- [ ] If applicable, have you updated the LICENSE file, including the main 
LICENSE file under nifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main 
NOTICE file found under nifi-assembly?
- [ ] If adding new Properties, have you added .displayName in addition to 
.name (programmatic access) for each of the new properties?

### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in 
which it is rendered?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/mattyb149/nifi NIFI-5082

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi/pull/2638.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2638


commit 739cccf468a505a8d9866e847fbbb2a37a24817a
Author: Matthew Burgess 
Date:   2018-04-17T14:44:26Z

NIFI-5082: Added support for custom Oracle timestamp types to Avro 
conversion




> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Assignee: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[Matt Burgess (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16438025#comment-16438025
 ] 

Matt Burgess commented on NIFI-5082:


We should probably also support TIMESTAMP WITH TIME ZONE since it is a valid 
SQL type (with value 2014). Unfortunately [Oracle has its own 
extensions|https://docs.oracle.com/cd/B13789_01/java.101/b10979/basic.htm#g1028145]
 and does not return 2014 for TIMESTAMP WITH TIME ZONE, it returns -101. In the 
default processing (where we currently just throw an IllegalArgumentException), 
we may want to check to see if the actual object is a Timestamp and perform the 
processing as such. However I am leery of doing this as it seems brittle and 
likely not cross-platform.

> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (NIFI-5082) SQL processors do not handle Avro conversion of Oracle timestamps correctly

[Matt Burgess (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-5082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16437506#comment-16437506
 ] 

Matt Burgess commented on NIFI-5082:


I propose we check the SQL type for TIMESTAMP (93), and if that is the case, we 
use getTimestamp() rather than getObject() to put into the "value" variable for 
further processing. 

> SQL processors do not handle Avro conversion of Oracle timestamps correctly
> ---
>
> Key: NIFI-5082
> URL: https://issues.apache.org/jira/browse/NIFI-5082
> Project: Apache NiFi
>  Issue Type: Bug
>Reporter: Matt Burgess
>Priority: Major
>
> In JdbcCommon (used by such processors as ExecuteSQL and QueryDatabaseTable), 
> if a ResultSet column is not a CLOB or BLOB, its value is retrieved using 
> getObject(), then further processing is done based on the SQL type and/or the 
> Java class of the value.
> However, in Oracle when getObject() is called on a Timestamp column, it 
> returns an Oracle-specific TIMESTAMP class which does not inherit from 
> java.sql.Timestamp or java.sql.Date. Thus the processing "falls through" and 
> its value is attempted to be inserted as a string, which violates the Avro 
> schema (which correctly recognized it as a long of timestamp logical type).
> At least for Oracle, the right way to process a Timestamp column is to call 
> getTimestamp() rather than getObject(), the former returns a 
> java.sql.Timestamp object which would correctly be processed by the current 
> code. I would hope that all drivers would support this but we would want to 
> test on (at least) MySQL, Oracle, and PostgreSQL.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)