Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed

Conversation

graben
Copy link
Contributor

@graben graben commented Feb 12, 2018

No description provided.

@franz1981
Copy link
Contributor

Please run the JDBC tests with Oracle 12 (now possible using the right sys properties) and fix the PR tile/message

@graben
Copy link
Contributor Author

graben commented Feb 15, 2018

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

@franz1981
Copy link
Contributor

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

@graben graben changed the title ARTEMIS-1660: Remove oracle12 autoincrement from column id for journa… ARTEMIS-1660: Remove oracle12 autoincrement from column id for journal tables Feb 15, 2018
@graben
Copy link
Contributor Author

graben commented Feb 15, 2018

Well, I'm actually not able to test against oracle 12 only have a oracle 11 express instance. 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.

@franz1981
Copy link
Contributor

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

@jmesnil
Copy link
Contributor

jmesnil commented Feb 16, 2018

FWIW, I copied the SQL statement from Artemis 2.4.0

private final String createJournalTableSQL = "CREATE TABLE " + tableName + " (id NUMBER(19) GENERATED BY DEFAULT ON NULL AS IDENTITY,recordType NUMBER(5),compactCount NUMBER(5),txId NUMBER(19),userRecordType NUMBER(5),variableSize NUMBER(10),record BLOB,txDataSize NUMBER(10),txData BLOB,txCheckNoRecords NUMBER(10),seq NUMBER(19))";

@graben
Copy link
Contributor Author

graben commented Feb 20, 2018

@mtaylor: Any feedback on this?

@franz1981
Copy link
Contributor

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

@asfgit asfgit closed this in d58c23c Mar 6, 2018
@graben graben deleted the ARTEMIS-1660 branch March 7, 2018 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants