[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-21 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
@graben @mtaylor was the right person that knows why the ID was configured 
in that way; I will check on the Oracle 12c doc about it in order to give you 
feedback ASAP :+1: 


---


[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-20 Thread graben
Github user graben commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
@mtaylor: Any feedback on this?


---


[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-16 Thread jmesnil
Github user jmesnil commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
FWIW, I copied the SQL statement from Artemis 2.4.0 
https://github.com/apache/activemq-artemis/blob/ec63189a0a8235fc0436d9554bcdf323944e1bf6/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/oracle/Oracle12CSQLProvider.java#L26



---


[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-16 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
@graben 
> Well, I'm actually not able to test against oracle 12 only have a oracle 
11 express instance

It would be enough: it is just to validate the changes.

> But it is obvious that the autoincrement of id is wrong, since it is 1st 
managed by the broker and 2nd different to all other database types.

Agree but would be anyway welcome at least a test round with the change.
I will ping @mtaylor and @jmesnil authors of most the SQL contained there 
to validate it: if they think that the change is safe as it seems, for me is ok 
:+1: 



---


[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-15 Thread graben
Github user graben commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
Well, I'm actually not able to test against oracle 12 only have a oracle 11 
express instance. But a it is obvious that the autoincrement of id is wrong, 
since it is 1st managed by the broker and 2nd different to all other database 
types.


---


[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-15 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
@graben 

> Please run the JDBC tests with Oracle 12 (now possible using the right 
sys properties)

Currently we're not supporting Oracle into the Jenkins CI hence please run 
the JDBC tests vs Oracle 12 to see if there is anything broken: in the latest 
master is possible to run all the tests against specific DBMS different from 
the embedded derby by using configurable system properties

>and fix the PR title/message

The PR title is: "ARTEMIS-1660: Remove oracle12 autoincrement from column 
id for journa…"
and the comment is: "…l tables"

Please fix both (ie title and message) in order to have a complete title 
and a more esplicit message :+1: 





---


[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-15 Thread graben
Github user graben commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
@franz1981 : sorry I don't understand what you would like to tell? Do you 
think the title is wrong? Do you think my patch is wrong?


---


[GitHub] activemq-artemis issue #1866: ARTEMIS-1660: Remove oracle12 autoincrement fr...

2018-02-15 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1866
  
Please run the JDBC tests with Oracle 12 (now possible using the right sys 
properties) and fix the PR tile/message


---