[jdbc] Add console command for checking/repairing schema integrity (#13765)

* Add console command for checking schema integrity
* Remove unneeded logging
* Add console command for fixing schema integrity
* Provide documentation
* Try to add support for Derby and PostgreSQL
* Sort alphabetically by item name

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

bundles/org.openhab.persistence.jdbc/README.md

@@ -208,6 +208,12 @@ Manual changes in the index table, `Items`, will not be picked up automatically
 The same is true when manually adding new item tables or deleting existing ones.
 After making such changes, the command `jdbc reload` can be used to reload the index.
 
+#### Check/fix Schema
+
+Use the command `jdbc schema check` to perform an integrity check of the schema.
+
+Identified issues can be fixed automatically using the command `jdbc schema fix` (all items having issues) or `jdbc schema fix <itemName>` (single item).
+
 ### For Developers
 
 * Clearly separated source files for the database-specific part of openHAB logic.

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

@@ -31,6 +31,7 @@ import org.openhab.core.persistence.FilterCriteria;
 import org.openhab.core.persistence.HistoricItem;
 import org.openhab.core.persistence.PersistenceItemInfo;
 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.dto.JdbcPersistenceItemInfo;
@@ -171,6 +172,17 @@ public class JdbcMapper {
         return vol;
     }
 
+    protected List<Column> getTableColumns(String tableName) throws JdbcSQLException {
+        logger.debug("JDBC::getTableColumns");
+        long timerStart = System.currentTimeMillis();
+        ItemsVO isvo = new ItemsVO();
+        isvo.setJdbcUriDatabaseName(conf.getDbName());
+        isvo.setTableName(tableName);
+        List<Column> is = conf.getDBDAO().doGetTableColumns(isvo);
+        logTime("getTableColumns", timerStart, System.currentTimeMillis());
+        return is;
+    }
+
     /****************
      * MAPPERS ITEM *
      ****************/
@@ -189,6 +201,14 @@ public class JdbcMapper {
         return vo;
     }
 
+    protected void alterTableColumn(String tableName, String columnName, String columnType, boolean nullable)
+            throws JdbcSQLException {
+        logger.debug("JDBC::alterTableColumn");
+        long timerStart = System.currentTimeMillis();
+        conf.getDBDAO().doAlterTableColumn(tableName, columnName, columnType, nullable);
+        logTime("alterTableColumn", timerStart, System.currentTimeMillis());
+    }
+
     protected void storeItemValue(Item item, State itemState, @Nullable ZonedDateTime date) throws JdbcException {
         logger.debug("JDBC::storeItemValue: item={} state={} date={}", item, itemState, date);
         String tableName = getTable(item);

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

@@ -40,6 +40,8 @@ import org.openhab.core.persistence.QueryablePersistenceService;
 import org.openhab.core.persistence.strategy.PersistenceStrategy;
 import org.openhab.core.types.State;
 import org.openhab.core.types.UnDefType;
+import org.openhab.persistence.jdbc.internal.db.JdbcBaseDAO;
+import org.openhab.persistence.jdbc.internal.dto.Column;
 import org.openhab.persistence.jdbc.internal.dto.ItemsVO;
 import org.openhab.persistence.jdbc.internal.exceptions.JdbcException;
 import org.openhab.persistence.jdbc.internal.exceptions.JdbcSQLException;
@@ -303,6 +305,109 @@ public class JdbcPersistenceService extends JdbcMapper implements ModifiablePers
         return itemNameToTableNameMap.keySet();
     }
 
+    /**
+     * Get a map of item names to table names.
+     */
+    public Map<String, String> getItemNameToTableNameMap() {
+        return itemNameToTableNameMap;
+    }
+
+    /**
+     * Check schema for integrity issues.
+     *
+     * @param tableName for which columns should be checked
+     * @param itemName that corresponds to table
+     * @return Collection of strings, each describing an identified issue
+     * @throws JdbcSQLException on SQL errors
+     */
+    public Collection<String> getSchemaIssues(String tableName, String itemName) throws JdbcSQLException {
+        List<String> issues = new ArrayList<>();
+        Item item;
+        try {
+            item = itemRegistry.getItem(itemName);
+        } catch (ItemNotFoundException e) {
+            return issues;
+        }
+        JdbcBaseDAO dao = conf.getDBDAO();
+        String timeDataType = dao.sqlTypes.get("tablePrimaryKey");
+        if (timeDataType == null) {
+            return issues;
+        }
+        String valueDataType = dao.getDataType(item);
+        List<Column> columns = getTableColumns(tableName);
+        for (Column column : columns) {
+            String columnName = column.getColumnName();
+            if ("time".equalsIgnoreCase(columnName)) {
+                if (!"time".equals(columnName)) {
+                    issues.add("Column name 'time' expected, but is '" + columnName + "'");
+                }
+                if (!timeDataType.equalsIgnoreCase(column.getColumnType())) {
+                    issues.add("Column type '" + timeDataType + "' expected, but is '"
+                            + column.getColumnType().toUpperCase() + "'");
+                }
+                if (column.getIsNullable()) {
+                    issues.add("Column 'time' expected to be NOT NULL, but is nullable");
+                }
+            } else if ("value".equalsIgnoreCase(columnName)) {
+                if (!"value".equals(columnName)) {
+                    issues.add("Column name 'value' expected, but is '" + columnName + "'");
+                }
+                if (!valueDataType.equalsIgnoreCase(column.getColumnType())) {
+                    issues.add("Column type '" + valueDataType + "' expected, but is '"
+                            + column.getColumnType().toUpperCase() + "'");
+                }
+                if (!column.getIsNullable()) {
+                    issues.add("Column 'value' expected to be nullable, but is NOT NULL");
+                }
+            } else {
+                issues.add("Column '" + columnName + "' not expected");
+            }
+        }
+        return issues;
+    }
+
+    /**
+     * Fix schema issues.
+     *
+     * @param tableName for which columns should be repaired
+     * @param itemName that corresponds to table
+     * @return true if table was altered, otherwise false
+     * @throws JdbcSQLException on SQL errors
+     */
+    public boolean fixSchemaIssues(String tableName, String itemName) throws JdbcSQLException {
+        Item item;
+        try {
+            item = itemRegistry.getItem(itemName);
+        } catch (ItemNotFoundException e) {
+            return false;
+        }
+        JdbcBaseDAO dao = conf.getDBDAO();
+        String timeDataType = dao.sqlTypes.get("tablePrimaryKey");
+        if (timeDataType == null) {
+            return false;
+        }
+        String valueDataType = dao.getDataType(item);
+        List<Column> columns = getTableColumns(tableName);
+        boolean isFixed = false;
+        for (Column column : columns) {
+            String columnName = column.getColumnName();
+            if ("time".equalsIgnoreCase(columnName)) {
+                if (!"time".equals(columnName) || !timeDataType.equalsIgnoreCase(column.getColumnType())
+                        || column.getIsNullable()) {
+                    alterTableColumn(tableName, "time", timeDataType, false);
+                    isFixed = true;
+                }
+            } else if ("value".equalsIgnoreCase(columnName)) {
+                if (!"value".equals(columnName) || !valueDataType.equalsIgnoreCase(column.getColumnType())
+                        || !column.getIsNullable()) {
+                    alterTableColumn(tableName, "value", valueDataType, true);
+                    isFixed = true;
+                }
+            }
+        }
+        return isFixed;
+    }
+
     /**
      * Get a list of all items with corresponding tables and an {@link ItemTableCheckEntryStatus} indicating
      * its condition.

bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/console/JdbcCommandExtension.java

@@ -13,8 +13,12 @@
 package org.openhab.persistence.jdbc.internal.console;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -44,13 +48,19 @@ import org.osgi.service.component.annotations.Reference;
 @Component(service = ConsoleCommandExtension.class)
 public class JdbcCommandExtension extends AbstractConsoleCommandExtension implements ConsoleCommandCompleter {
 
+    private static final String CMD_SCHEMA = "schema";
     private static final String CMD_TABLES = "tables";
     private static final String CMD_RELOAD = "reload";
+    private static final String SUBCMD_SCHEMA_CHECK = "check";
+    private static final String SUBCMD_SCHEMA_FIX = "fix";
     private static final String SUBCMD_TABLES_LIST = "list";
     private static final String SUBCMD_TABLES_CLEAN = "clean";
     private static final String PARAMETER_ALL = "all";
     private static final String PARAMETER_FORCE = "force";
-    private static final StringsCompleter CMD_COMPLETER = new StringsCompleter(List.of(CMD_TABLES, CMD_RELOAD), false);
+    private static final StringsCompleter CMD_COMPLETER = new StringsCompleter(
+            List.of(CMD_SCHEMA, CMD_TABLES, CMD_RELOAD), false);
+    private static final StringsCompleter SUBCMD_SCHEMA_COMPLETER = new StringsCompleter(
+            List.of(SUBCMD_SCHEMA_CHECK, SUBCMD_SCHEMA_FIX), false);
     private static final StringsCompleter SUBCMD_TABLES_COMPLETER = new StringsCompleter(
             List.of(SUBCMD_TABLES_LIST, SUBCMD_TABLES_CLEAN), false);
 
@@ -109,6 +119,19 @@ public class JdbcCommandExtension extends AbstractConsoleCommandExtension implem
                     return true;
                 }
             }
+        } else if (args.length > 1 && CMD_SCHEMA.equalsIgnoreCase(args[0])) {
+            if (args.length == 2 && SUBCMD_SCHEMA_CHECK.equalsIgnoreCase(args[1])) {
+                checkSchema(persistenceService, console);
+                return true;
+            } else if (SUBCMD_SCHEMA_FIX.equalsIgnoreCase(args[1])) {
+                if (args.length == 2) {
+                    fixSchema(persistenceService, console);
+                    return true;
+                } else if (args.length == 3) {
+                    fixSchema(persistenceService, console, args[2]);
+                    return true;
+                }
+            }
         } else if (args.length == 1 && CMD_RELOAD.equalsIgnoreCase(args[0])) {
             reload(persistenceService, console);
             return true;
@@ -116,7 +139,62 @@ public class JdbcCommandExtension extends AbstractConsoleCommandExtension implem
         return false;
     }
 
-    private void listTables(JdbcPersistenceService persistenceService, Console console, Boolean all)
+    private void checkSchema(JdbcPersistenceService persistenceService, Console console) throws JdbcSQLException {
+        List<Entry<String, String>> itemNameToTableName = persistenceService.getItemNameToTableNameMap().entrySet()
+                .stream().sorted(Map.Entry.comparingByKey()).collect(Collectors.toList());
+        int itemNameMaxLength = Math
+                .max(itemNameToTableName.stream().map(i -> i.getKey().length()).max(Integer::compare).orElse(0), 4);
+        int tableNameMaxLength = Math
+                .max(itemNameToTableName.stream().map(i -> i.getValue().length()).max(Integer::compare).orElse(0), 5);
+        console.println(String.format("%1$-" + (tableNameMaxLength + 2) + "s%2$-" + (itemNameMaxLength + 2) + "s%3$s",
+                "Table", "Item", "Issue"));
+        console.println("-".repeat(tableNameMaxLength) + "  " + "-".repeat(itemNameMaxLength) + "  " + "-".repeat(64));
+        for (Entry<String, String> entry : itemNameToTableName) {
+            String itemName = entry.getKey();
+            String tableName = entry.getValue();
+            Collection<String> issues = persistenceService.getSchemaIssues(tableName, itemName);
+            if (!issues.isEmpty()) {
+                for (String issue : issues) {
+                    console.println(String.format(
+                            "%1$-" + (tableNameMaxLength + 2) + "s%2$-" + (itemNameMaxLength + 2) + "s%3$s", tableName,
+                            itemName, issue));
+                }
+            }
+        }
+    }
+
+    private void fixSchema(JdbcPersistenceService persistenceService, Console console) {
+        List<Entry<String, String>> itemNameToTableName = persistenceService.getItemNameToTableNameMap().entrySet()
+                .stream().sorted(Map.Entry.comparingByKey()).collect(Collectors.toList());
+        for (Entry<String, String> entry : itemNameToTableName) {
+            String itemName = entry.getKey();
+            String tableName = entry.getValue();
+            fixSchema(persistenceService, console, tableName, itemName);
+        }
+    }
+
+    private void fixSchema(JdbcPersistenceService persistenceService, Console console, String itemName) {
+        Map<String, String> itemNameToTableNameMap = persistenceService.getItemNameToTableNameMap();
+        String tableName = itemNameToTableNameMap.get(itemName);
+        if (tableName != null) {
+            fixSchema(persistenceService, console, tableName, itemName);
+        } else {
+            console.println("Table not found for item '" + itemName + "'");
+        }
+    }
+
+    private void fixSchema(JdbcPersistenceService persistenceService, Console console, String tableName,
+            String itemName) {
+        try {
+            if (persistenceService.fixSchemaIssues(tableName, itemName)) {
+                console.println("Fixed table '" + tableName + "' for item '" + itemName + "'");
+            }
+        } catch (JdbcSQLException e) {
+            console.println("Failed to fix table '" + tableName + "' for item '" + itemName + "': " + e.getMessage());
+        }
+    }
+
+    private void listTables(JdbcPersistenceService persistenceService, Console console, boolean all)
             throws JdbcSQLException {
         List<ItemTableCheckEntry> entries = persistenceService.getCheckedEntries();
         if (!all) {
@@ -176,7 +254,8 @@ public class JdbcCommandExtension extends AbstractConsoleCommandExtension implem
 
     @Override
     public List<String> getUsages() {
-        return Arrays.asList(
+        return Arrays.asList(buildCommandUsage(CMD_SCHEMA + " " + SUBCMD_SCHEMA_CHECK, "check schema integrity"),
+                buildCommandUsage(CMD_SCHEMA + " " + SUBCMD_SCHEMA_FIX + " [<itemName>]", "fix schema integrity"),
                 buildCommandUsage(CMD_TABLES + " " + SUBCMD_TABLES_LIST + " [" + PARAMETER_ALL + "]",
                         "list tables (all = include valid)"),
                 buildCommandUsage(
@@ -197,6 +276,8 @@ public class JdbcCommandExtension extends AbstractConsoleCommandExtension implem
         } else if (cursorArgumentIndex == 1) {
             if (CMD_TABLES.equalsIgnoreCase(args[0])) {
                 return SUBCMD_TABLES_COMPLETER.complete(args, cursorArgumentIndex, cursorPosition, candidates);
+            } else if (CMD_SCHEMA.equalsIgnoreCase(args[0])) {
+                return SUBCMD_SCHEMA_COMPLETER.complete(args, cursorArgumentIndex, cursorPosition, candidates);
             }
         } else if (cursorArgumentIndex == 2) {
             if (CMD_TABLES.equalsIgnoreCase(args[0])) {
@@ -210,6 +291,14 @@ public class JdbcCommandExtension extends AbstractConsoleCommandExtension implem
                     new StringsCompleter(List.of(PARAMETER_ALL), false).complete(args, cursorArgumentIndex,
                             cursorPosition, candidates);
                 }
+            } else if (CMD_SCHEMA.equalsIgnoreCase(args[0])) {
+                if (SUBCMD_SCHEMA_FIX.equalsIgnoreCase(args[1])) {
+                    JdbcPersistenceService persistenceService = getPersistenceService();
+                    if (persistenceService != null) {
+                        return new StringsCompleter(persistenceService.getItemNames(), true).complete(args,
+                                cursorArgumentIndex, cursorPosition, candidates);
+                    }
+                }
             }
         }
         return false;

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

@@ -56,6 +56,7 @@ import org.openhab.core.persistence.FilterCriteria.Ordering;
 import org.openhab.core.persistence.HistoricItem;
 import org.openhab.core.types.State;
 import org.openhab.core.types.TypeParser;
+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.dto.JdbcHistoricItem;
@@ -91,8 +92,10 @@ public class JdbcBaseDAO {
     protected String sqlDeleteItemsEntry = "DELETE FROM #itemsManageTable# WHERE ItemName='#itemname#'";
     protected String sqlGetItemIDTableNames = "SELECT ItemId, ItemName FROM #itemsManageTable#";
     protected String sqlGetItemTables = "SELECT table_name FROM information_schema.tables WHERE table_type='BASE TABLE' AND table_schema='#jdbcUriDatabaseName#' AND NOT table_name='#itemsManageTable#'";
+    protected String sqlGetTableColumnTypes = "SELECT column_name, column_type, is_nullable FROM information_schema.columns WHERE table_schema='#jdbcUriDatabaseName#' AND table_name='#tableName#'";
     protected String sqlCreateItemTable = "CREATE TABLE IF NOT EXISTS #tableName# (time #tablePrimaryKey# NOT NULL, value #dbType#, PRIMARY KEY(time))";
-    protected String sqlInsertItemValue = "INSERT INTO #tableName# (TIME, VALUE) VALUES( #tablePrimaryValue#, ? ) ON DUPLICATE KEY UPDATE VALUE= ?";
+    protected String sqlAlterTableColumn = "ALTER TABLE #tableName# MODIFY COLUMN #columnName# #columnType#";
+    protected String sqlInsertItemValue = "INSERT INTO #tableName# (time, value) VALUES( #tablePrimaryValue#, ? ) ON DUPLICATE KEY UPDATE VALUE= ?";
     protected String sqlGetRowCount = "SELECT COUNT(*) FROM #tableName#";
 
     /********
@@ -375,6 +378,18 @@ public class JdbcBaseDAO {
         }
     }
 
+    public List<Column> doGetTableColumns(ItemsVO vo) throws JdbcSQLException {
+        String sql = StringUtilsExt.replaceArrayMerge(sqlGetTableColumnTypes,
+                new String[] { "#jdbcUriDatabaseName#", "#tableName#" },
+                new String[] { vo.getJdbcUriDatabaseName(), vo.getTableName() });
+        logger.debug("JDBC::doGetTableColumns sql={}", sql);
+        try {
+            return Yank.queryBeanList(sql, Column.class, null);
+        } catch (YankSQLException e) {
+            throw new JdbcSQLException(e);
+        }
+    }
+
     /*************
      * ITEM DAOs *
      *************/
@@ -402,6 +417,19 @@ public class JdbcBaseDAO {
         }
     }
 
+    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, nullable ? columnType : columnType + " NOT NULL" });
+        logger.debug("JDBC::doAlterTableColumn sql={}", sql);
+        try {
+            Yank.execute(sql, null);
+        } catch (YankSQLException e) {
+            throw new JdbcSQLException(e);
+        }
+    }
+
     public void doStoreItemValue(Item item, State itemState, ItemVO vo) throws JdbcSQLException {
         ItemVO storedVO = storeItemValueProvider(item, itemState, vo);
         String sql = StringUtilsExt.replaceArrayMerge(sqlInsertItemValue,
@@ -727,7 +755,6 @@ public class JdbcBaseDAO {
             }
         }
         String itemType = item.getClass().getSimpleName().toUpperCase();
-        logger.debug("JDBC::getItemType: Try to use ItemType {} for Item {}", itemType, i.getName());
         if (sqlTypes.get(itemType) == null) {
             logger.warn(
                     "JDBC::getItemType: No sqlType found for ItemType {}, use ItemType for STRINGITEM as Fallback for {}",

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

@@ -72,6 +72,7 @@ public class JdbcDerbyDAO extends JdbcBaseDAO {
         // Prevent error against duplicate time value (seldom): No powerful Merge found:
         // http://www.codeproject.com/Questions/162627/how-to-insert-new-record-in-my-table-if-not-exists
         sqlInsertItemValue = "INSERT INTO #tableName# (TIME, VALUE) VALUES( #tablePrimaryValue#, CAST( ? as #dbType#) )";
+        sqlAlterTableColumn = "ALTER TABLE #tableName# ALTER COLUMN #columnName# SET DATA TYPE #columnType#";
     }
 
     private void initSqlTypes() {

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

@@ -68,6 +68,7 @@ public class JdbcPostgresqlDAO extends JdbcBaseDAO {
         // SQL_INSERT_ITEM_VALUE = "INSERT INTO #tableName# (TIME, VALUE) VALUES( NOW(), CAST( ? as #dbType#) ) ON
         // CONFLICT DO NOTHING";
         sqlInsertItemValue = "INSERT INTO #tableName# (TIME, VALUE) VALUES( #tablePrimaryValue#, CAST( ? as #dbType#) )";
+        sqlAlterTableColumn = "ALTER TABLE #tableName# ALTER COLUMN #columnName# TYPE #columnType#";
     }
 
     /**

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

@@ -0,0 +1,55 @@
+/**
+ * Copyright (c) 2010-2022 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.persistence.jdbc.internal.dto;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+
+/**
+ * Represents an INFORMATON_SCHEMA.COLUMNS table row.
+ *
+ * @author Jacob Laursen - Initial contribution
+ */
+@NonNullByDefault
+public class Column {
+
+    private @Nullable String columnName;
+    private boolean isNullable;
+    private @Nullable String columnType;
+
+    public String getColumnName() {
+        String columnName = this.columnName;
+        return columnName != null ? columnName : "";
+    }
+
+    public String getColumnType() {
+        String columnType = this.columnType;
+        return columnType != null ? columnType : "";
+    }
+
+    public boolean getIsNullable() {
+        return isNullable;
+    }
+
+    public void setColumnName(String columnName) {
+        this.columnName = columnName;
+    }
+
+    public void setColumnType(String columnType) {
+        this.columnType = columnType;
+    }
+
+    public void setIsNullable(boolean isNullable) {
+        this.isNullable = isNullable;
+    }
+}