Working PostgreSQL Schema check/fix and set TIMESTAMPTZ to match MySQL defaults (#14294)

Signed-off-by: Ilias Kotinas <henfiber@gmail.com>

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcMapper.java

@@ -180,6 +180,7 @@ public class JdbcMapper {
         ItemsVO isvo = new ItemsVO();
         isvo.setJdbcUriDatabaseName(conf.getDbName());
         isvo.setTableName(tableName);
+        isvo.setItemsManageTable(conf.getItemsManageTable());
         List<Column> is = conf.getDBDAO().doGetTableColumns(isvo);
         logTime("getTableColumns", timerStart, System.currentTimeMillis());
         return is;

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcPersistenceService.java

@@ -313,7 +313,7 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
     }
 
     /**
-     * Check schema for integrity issues.
+     * Check schema of specific item table for integrity issues.
      *
      * @param tableName for which columns should be checked
      * @param itemName that corresponds to table
@@ -347,7 +347,8 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
                 if (!"time".equals(columnName)) {
                     issues.add("Column name 'time' expected, but is '" + columnName + "'");
                 }
-                if (!timeDataType.equalsIgnoreCase(column.getColumnType())) {
+                if (!timeDataType.equalsIgnoreCase(column.getColumnType())
+                        && !timeDataType.equalsIgnoreCase(column.getColumnTypeAlias())) {
                     issues.add("Column type '" + timeDataType + "' expected, but is '"
                             + column.getColumnType().toUpperCase() + "'");
                 }
@@ -358,7 +359,8 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
                 if (!"value".equals(columnName)) {
                     issues.add("Column name 'value' expected, but is '" + columnName + "'");
                 }
-                if (!valueDataType.equalsIgnoreCase(column.getColumnType())) {
+                if (!valueDataType.equalsIgnoreCase(column.getColumnType())
+                        && !valueDataType.equalsIgnoreCase(column.getColumnTypeAlias())) {
                     issues.add("Column type '" + valueDataType + "' expected, but is '"
                             + column.getColumnType().toUpperCase() + "'");
                 }
@@ -403,13 +405,17 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
         for (Column column : columns) {
             String columnName = column.getColumnName();
             if ("time".equalsIgnoreCase(columnName)) {
-                if (!"time".equals(columnName) || !timeDataType.equalsIgnoreCase(column.getColumnType())
+                if (!"time".equals(columnName)
+                        || (!timeDataType.equalsIgnoreCase(column.getColumnType())
+                                && !timeDataType.equalsIgnoreCase(column.getColumnTypeAlias()))
                         || column.getIsNullable()) {
                     alterTableColumn(tableName, "time", timeDataType, false);
                     isFixed = true;
                 }
             } else if ("value".equalsIgnoreCase(columnName)) {
-                if (!"value".equals(columnName) || !valueDataType.equalsIgnoreCase(column.getColumnType())
+                if (!"value".equals(columnName)
+                        || (!valueDataType.equalsIgnoreCase(column.getColumnType())
+                                && !valueDataType.equalsIgnoreCase(column.getColumnTypeAlias()))
                         || !column.getIsNullable()) {
                     alterTableColumn(tableName, "value", valueDataType, true);
                     isFixed = true;

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/db/JdbcPostgresqlDAO.java

@@ -23,6 +23,7 @@ import org.openhab.core.items.Item;
 import org.openhab.core.persistence.FilterCriteria;
 import org.openhab.core.persistence.FilterCriteria.Ordering;
 import org.openhab.core.types.State;
+import org.openhab.persistence.jdbc.internal.dto.Column;
 import org.openhab.persistence.jdbc.internal.dto.ItemVO;
 import org.openhab.persistence.jdbc.internal.dto.ItemsVO;
 import org.openhab.persistence.jdbc.internal.exceptions.JdbcSQLException;
@@ -64,6 +65,12 @@ public class JdbcPostgresqlDAO extends JdbcBaseDAO {
         sqlCreateNewEntryInItemsTable = "INSERT INTO items (itemname) SELECT itemname FROM #itemsManageTable# UNION VALUES ('#itemname#') EXCEPT SELECT itemname FROM items";
         sqlGetItemTables = "SELECT table_name FROM information_schema.tables WHERE table_type='BASE TABLE' AND table_schema=(SELECT table_schema "
                 + "FROM information_schema.tables WHERE table_type='BASE TABLE' AND table_name='#itemsManageTable#') AND NOT table_name='#itemsManageTable#'";
+        // The PostgreSQL equivalent to MySQL columns.column_type is data_type (e.g. "timestamp with time zone") and
+        // udt_name which contains a shorter alias (e.g. "timestamptz"). We alias data_type as "column_type" and
+        // udt_name as "column_type_alias" to be compatible with the 'Column' class used in Yank.queryBeanList
+        sqlGetTableColumnTypes = "SELECT column_name, data_type as column_type, udt_name as column_type_alias, is_nullable FROM information_schema.columns "
+                + "WHERE table_name='#tableName#' AND table_catalog='#jdbcUriDatabaseName#' AND table_schema=(SELECT table_schema FROM information_schema.tables WHERE table_type='BASE TABLE' "
+                + "AND table_name='#itemsManageTable#')";
         // NOTICE: on PostgreSql >= 9.5, sqlInsertItemValue query template is modified to do an "upsert" (overwrite
         // existing value). The version check and query change is performed at initAfterFirstDbConnection()
         sqlInsertItemValue = "INSERT INTO #tableName# (TIME, VALUE) VALUES( #tablePrimaryValue#, CAST( ? as #dbType#) )";
@@ -93,7 +100,7 @@ public class JdbcPostgresqlDAO extends JdbcBaseDAO {
         sqlTypes.put("CALLITEM", "VARCHAR");
         sqlTypes.put("COLORITEM", "VARCHAR");
         sqlTypes.put("CONTACTITEM", "VARCHAR");
-        sqlTypes.put("DATETIMEITEM", "TIMESTAMP");
+        sqlTypes.put("DATETIMEITEM", "TIMESTAMPTZ");
         sqlTypes.put("DIMMERITEM", "SMALLINT");
         sqlTypes.put("IMAGEITEM", "VARCHAR");
         sqlTypes.put("LOCATIONITEM", "VARCHAR");
@@ -102,6 +109,7 @@ public class JdbcPostgresqlDAO extends JdbcBaseDAO {
         sqlTypes.put("ROLLERSHUTTERITEM", "SMALLINT");
         sqlTypes.put("STRINGITEM", "VARCHAR");
         sqlTypes.put("SWITCHITEM", "VARCHAR");
+        sqlTypes.put("tablePrimaryKey", "TIMESTAMPTZ");
         logger.debug("JDBC::initSqlTypes: Initialized the type array sqlTypes={}", sqlTypes.values());
     }
 
@@ -165,9 +173,50 @@ public class JdbcPostgresqlDAO extends JdbcBaseDAO {
         }
     }
 
+    /*
+     * Override because for PostgreSQL a different query is required with a 3rd argument (itemsManageTable)
+     */
+    @Override
+    public List<Column> doGetTableColumns(ItemsVO vo) throws JdbcSQLException {
+        String sql = StringUtilsExt.replaceArrayMerge(sqlGetTableColumnTypes,
+                new String[] { "#jdbcUriDatabaseName#", "#tableName#", "#itemsManageTable#" },
+                new String[] { vo.getJdbcUriDatabaseName(), vo.getTableName(), vo.getItemsManageTable() });
+        logger.debug("JDBC::doGetTableColumns sql={}", sql);
+        try {
+            return Yank.queryBeanList(sql, Column.class, null);
+        } catch (YankSQLException e) {
+            throw new JdbcSQLException(e);
+        }
+    }
+
     /*************
      * ITEM DAOs *
      *************/
+
+    /*
+     * Override since PostgreSQL does not support setting NOT NULL in the same clause as ALTER COLUMN .. TYPE
+     */
+    @Override
+    public void doAlterTableColumn(String tableName, String columnName, String columnType, boolean nullable)
+            throws JdbcSQLException {
+        String sql = StringUtilsExt.replaceArrayMerge(sqlAlterTableColumn,
+                new String[] { "#tableName#", "#columnName#", "#columnType#" },
+                new String[] { tableName, columnName, columnType });
+        logger.info("JDBC::doAlterTableColumn sql={}", sql);
+        try {
+            Yank.execute(sql, null);
+            if (!nullable) {
+                String sql2 = StringUtilsExt.replaceArrayMerge(
+                        "ALTER TABLE #tableName# ALTER COLUMN #columnName# SET NOT NULL",
+                        new String[] { "#tableName#", "#columnName#" }, new String[] { tableName, columnName });
+                logger.info("JDBC::doAlterTableColumn sql={}", sql2);
+                Yank.execute(sql2, null);
+            }
+        } catch (YankSQLException e) {
+            throw new JdbcSQLException(e);
+        }
+    }
+
     @Override
     public void doStoreItemValue(Item item, State itemState, ItemVO vo) throws JdbcSQLException {
         ItemVO storedVO = storeItemValueProvider(item, itemState, vo);

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/dto/Column.java

@@ -18,6 +18,12 @@ import org.eclipse.jdt.annotation.Nullable;
 /**
  * Represents an INFORMATON_SCHEMA.COLUMNS table row.
  *
+ * MySQL returns type as column_type
+ *
+ * PostgreSQL returns "data_type" (e.g. "character varying") and "udt_name" as a type alias (e.g. "varchar")
+ * these should be aliased as the matching snake_case version of the attributes in this class. i.e.:
+ * SELECT column_name, data_type as column_type, udt_name as column_type_alias FROM information_schema.columns
+ *
  * @author Jacob Laursen - Initial contribution
  */
 @NonNullByDefault
@@ -26,6 +32,7 @@ public class Column {
     private @Nullable String columnName;
     private boolean isNullable;
     private @Nullable String columnType;
+    private @Nullable String columnTypeAlias;
 
     public String getColumnName() {
         String columnName = this.columnName;
@@ -37,6 +44,11 @@ public class Column {
         return columnType != null ? columnType : "";
     }
 
+    public String getColumnTypeAlias() {
+        String columnTypeAlias = this.columnTypeAlias;
+        return columnTypeAlias != null ? columnTypeAlias : "";
+    }
+
     public boolean getIsNullable() {
         return isNullable;
     }
@@ -49,6 +61,10 @@ public class Column {
         this.columnType = columnType;
     }
 
+    public void setColumnTypeAlias(String columnTypeAlias) {
+        this.columnTypeAlias = columnTypeAlias;
+    }
+
     public void setIsNullable(boolean isNullable) {
         this.isNullable = isNullable;
     }