[GitHub] activemq-artemis pull request #1986: ARTEMIS-1653 Allow database tables to b...

[franz1981]

Github user franz1981 closed the pull request at:

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


---

[GitHub] activemq-artemis pull request #1986: ARTEMIS-1653 Allow database tables to b...

[franz1981]

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

https://github.com/apache/activemq-artemis/pull/1986#discussion_r179441774
  
--- Diff: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/AbstractJDBCDriver.java
 ---
@@ -191,9 +191,18 @@ private void createTableIfNotExists(String tableName, 
String... sqls) throws SQL
 } else {
try (Statement statement = connection.createStatement();
  ResultSet cntRs = 
statement.executeQuery(sqlProvider.getCountJournalRecordsSQL())) {
-  if (rs.next() && rs.getInt(1) > 0) {
+  int rows;
+  if (cntRs.next() && (rows = cntRs.getInt(1)) > 0) {
  logger.tracef("Table %s did exist but is not empty. 
Skipping initialization.", tableName);
+ if (logger.isDebugEnabled()) {
+final long expectedRows = 
Stream.of(sqls).map(String::toUpperCase).filter(sql -> sql.contains("INSERT 
INTO")).count();
--- End diff --

@clebertsuconic PLease do not merge it yet...because on DB2 it is creating 
some problems


---

[GitHub] activemq-artemis pull request #1986: ARTEMIS-1653 Allow database tables to b...

[franz1981]

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

https://github.com/apache/activemq-artemis/pull/1986#discussion_r179124397
  
--- Diff: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/AbstractJDBCDriver.java
 ---
@@ -191,9 +191,18 @@ private void createTableIfNotExists(String tableName, 
String... sqls) throws SQL
 } else {
try (Statement statement = connection.createStatement();
  ResultSet cntRs = 
statement.executeQuery(sqlProvider.getCountJournalRecordsSQL())) {
-  if (rs.next() && rs.getInt(1) > 0) {
+  int rows;
+  if (cntRs.next() && (rows = cntRs.getInt(1)) > 0) {
  logger.tracef("Table %s did exist but is not empty. 
Skipping initialization.", tableName);
+ if (logger.isDebugEnabled()) {
+final long expectedRows = 
Stream.of(sqls).map(String::toUpperCase).filter(sql -> sql.contains("INSERT 
INTO")).count();
--- End diff --

Nope, I meant that the check should be performed only with debug log level 
to help spotting weird behavious.
It is not the most robust check ever, but it is pretty helpfull for HA (for 
example).


---

[GitHub] activemq-artemis pull request #1986: ARTEMIS-1653 Allow database tables to b...

[clebertsuconic]

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

https://github.com/apache/activemq-artemis/pull/1986#discussion_r178806417
  
--- Diff: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/drivers/AbstractJDBCDriver.java
 ---
@@ -191,9 +191,18 @@ private void createTableIfNotExists(String tableName, 
String... sqls) throws SQL
 } else {
try (Statement statement = connection.createStatement();
  ResultSet cntRs = 
statement.executeQuery(sqlProvider.getCountJournalRecordsSQL())) {
-  if (rs.next() && rs.getInt(1) > 0) {
+  int rows;
+  if (cntRs.next() && (rows = cntRs.getInt(1)) > 0) {
  logger.tracef("Table %s did exist but is not empty. 
Skipping initialization.", tableName);
+ if (logger.isDebugEnabled()) {
+final long expectedRows = 
Stream.of(sqls).map(String::toUpperCase).filter(sql -> sql.contains("INSERT 
INTO")).count();
--- End diff --

Are you sure you meant this INSERT to only happen if debug?


---

[GitHub] activemq-artemis pull request #1986: ARTEMIS-1653 Allow database tables to b...

[franz1981]

GitHub user franz1981 opened a pull request:

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

ARTEMIS-1653 Allow database tables to be created externally

The previous commit about this feature wasn't using the right ResultSet
to count the rows in a table.
Additional checks have been added to:
- verify that the first SQL statement is a CREATE TABLE
- log if the existing rows are less than the expected ones

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

$ git pull https://github.com/franz1981/activemq-artemis ARTEMIS-1653

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

https://github.com/apache/activemq-artemis/pull/1986.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 #1986


commit 66c442dd6a4eae02f7ac0d84eb0bffd6df9a01b3
Author: Francesco Nigro 
Date:   2018-04-03T08:11:04Z

ARTEMIS-1653 Allow database tables to be created externally

The previous commit about this feature wasn't using the right ResultSet
to count the rows in a table.
Additional checks have been added to:
- verify that the first SQL statement is a CREATE TABLE
- log if the existing rows are less than the expected ones




---