[GitHub] activemq-artemis pull request #1866: ARTEMIS-1660: Remove oracle12 autoincre...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1866: ARTEMIS-1660: Remove oracle12 autoincre...

jbertram-2
GitHub user graben opened a pull request:

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

    ARTEMIS-1660: Remove oracle12 autoincrement from column id for journa…

    …l tables

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

    $ git pull https://github.com/graben/activemq-artemis ARTEMIS-1660

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

    https://github.com/apache/activemq-artemis/pull/1866.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 #1866
   
----
commit 60e209b17a62f8b4d066f068fe057da43c740ecb
Author: Benjamin Graf <benjamin.graf@...>
Date:   2018-02-12T19:56:20Z

    ARTEMIS-1660: Remove oracle12 autoincrement from column id for journal tables

----


---
Reply | Threaded
Open this post in threaded view
|

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

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


---
Reply | Threaded
Open this post in threaded view
|

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

jbertram-2
In reply to this post by jbertram-2
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?


---
Reply | Threaded
Open this post in threaded view
|

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

jbertram-2
In reply to this post by jbertram-2
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:
   
   



---
Reply | Threaded
Open this post in threaded view
|

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

jbertram-2
In reply to this post by jbertram-2
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.


---
Reply | Threaded
Open this post in threaded view
|

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

jbertram-2
In reply to this post by jbertram-2
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:



---
Reply | Threaded
Open this post in threaded view
|

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

jbertram-2
In reply to this post by jbertram-2
Reply | Threaded
Open this post in threaded view
|

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

jbertram-2
In reply to this post by jbertram-2
Github user graben commented on the issue:

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


---
Reply | Threaded
Open this post in threaded view
|

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

jbertram-2
In reply to this post by jbertram-2
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:


---