[jdbc] Fix case-sensitive table names for PostgreSQL

[jlaur]

This eliminates duplicated queries for PostgreSQL and fixes some regressions as well.

Additionally make Derby queries consistent by always explicitly uppercasing the table names. These queries had it done already:

• sqlCreateItemsTableIfNot:

  CREATE TABLE #itemsManageTable# ( ItemId INTEGER NOT NULL GENERATED ALWAYS AS IDENTITY (START WITH 1, INCREMENT BY 1), #colname# #coltype# NOT NULL)

• sqlCreateNewEntryInItemsTable:

  INSERT INTO #itemsManageTable# (ItemName) VALUES ('#itemname#')

• sqlCreateItemsTableIfNot:

  CREATE TABLE IF NOT EXISTS #itemsManageTable# (ItemId INT NOT NULL AUTO_INCREMENT,#colname# #coltype# NOT NULL,PRIMARY KEY (ItemId))

• sqlInsertItemValue:

  INSERT INTO #tableName# (TIME, VALUE) VALUES( #tablePrimaryValue#, CAST( ? as #dbType#) )

While these did not:

• sqlDropItemsTableIfExists:

  DROP TABLE IF EXISTS #itemsManageTable#

• sqlDropTable:

  DROP TABLE #tableName#

• sqlDeleteItemsEntry:

  DELETE FROM #itemsManageTable# WHERE ItemName='#itemname#'

• sqlGetItemIDTableNames:

  SELECT ItemId, ItemName FROM #itemsManageTable#

• sqlCreateItemTable:

  CREATE TABLE #tableName# (time #tablePrimaryKey# NOT NULL, value #dbType#, PRIMARY KEY(time))

• sqlAlterTableColumn:

  ALTER TABLE #tableName# ALTER COLUMN #columnName# SET DATA TYPE #columnType#

Derby automatically uppercases unquoted table names, so these differences probably did not matter. Adding tableCaseSensitiveItemNames support would be possible in the same way as for PostgreSQL by using double quotes, but since all tables are already uppercased, this would not be backwards compatible, or at least it would be hard to ensure and properly migrate. Therefore it's out of scope for this PR, since I cannot test this. The Derby changes are untested.

Fixes #14671
Reverts and fixes regressions from #17587
Supersedes #17593

[jlaur]

@JonathanvdGHU - will you be able to test this with PostgreSQL?

[JonathanvdGHU]

@JonathanvdGHU - will you be able to test this with PostgreSQL?

I've tested it for TimescaleDB/PostgreSQL. Everything works fine when tableCaseSensitiveItemNames is on true. But I get the following errors when I set it on false:

2024-10-21 09:06:59.159 [ERROR] [jdbc.internal.JdbcPersistenceService] - Failed to check database schema
org.openhab.persistence.jdbc.internal.exceptions.JdbcSQLException: Error in SQL query!!!; ERROR: syntax error at or near "_pkey"
  Position: 111 Query: CREATE TABLE IF NOT EXISTS "items" (itemid SERIAL NOT NULL, ItemName VARCHAR(500) NOT NULL, CONSTRAINT "items"_pkey PRIMARY KEY (itemid)) Parameters: []; Pool Name= yank-default; SQL= CREATE TABLE IF NOT EXISTS "items" (itemid SERIAL NOT NULL, ItemName VARCHAR(500) NOT NULL, CONSTRAINT "items"_pkey PRIMARY KEY (itemid))
	at org.openhab.persistence.jdbc.internal.db.JdbcPostgresqlDAO.doCreateItemsTableIfNot(JdbcPostgresqlDAO.java:152) ~[?:?]
	at org.openhab.persistence.jdbc.internal.JdbcMapper.createItemsTableIfNot(JdbcMapper.java:128) ~[?:?]
	at org.openhab.persistence.jdbc.internal.JdbcMapper.checkDBSchema(JdbcMapper.java:321) ~[?:?]
	at org.openhab.persistence.jdbc.internal.JdbcPersistenceService.updateConfig(JdbcPersistenceService.java:263) ~[?:?]
	at org.openhab.persistence.jdbc.internal.JdbcPersistenceService.activate(JdbcPersistenceService.java:100) ~[?:?]
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[?:?]
	at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
	at java.lang.reflect.Method.invoke(Method.java:569) ~[?:?]
	at org.apache.felix.scr.impl.inject.methods.BaseMethod.invokeMethod(BaseMethod.java:245) ~[?:?]
	at org.apache.felix.scr.impl.inject.methods.BaseMethod.access$500(BaseMethod.java:41) ~[?:?]
	at org.apache.felix.scr.impl.inject.methods.BaseMethod$Resolved.invoke(BaseMethod.java:687) ~[?:?]
	at org.apache.felix.scr.impl.inject.methods.BaseMethod.invoke(BaseMethod.java:531) ~[?:?]
	at org.apache.felix.scr.impl.inject.methods.ActivateMethod.invoke(ActivateMethod.java:317) ~[?:?]
	at org.apache.felix.scr.impl.inject.methods.ActivateMethod.invoke(ActivateMethod.java:307) ~[?:?]
	at org.apache.felix.scr.impl.manager.SingleComponentManager.createImplementationObject(SingleComponentManager.java:354) ~[?:?]
	at org.apache.felix.scr.impl.manager.SingleComponentManager.createComponent(SingleComponentManager.java:115) ~[?:?]
	at org.apache.felix.scr.impl.manager.SingleComponentManager.getService(SingleComponentManager.java:1002) ~[?:?]
	at org.apache.felix.scr.impl.manager.SingleComponentManager.getServiceInternal(SingleComponentManager.java:975) ~[?:?]
	at org.apache.felix.scr.impl.manager.SingleComponentManager.getService(SingleComponentManager.java:920) ~[?:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceFactoryUse$1.run(ServiceFactoryUse.java:220) ~[org.eclipse.osgi-3.18.0.jar:?]
	at java.security.AccessController.doPrivileged(AccessController.java:318) [?:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceFactoryUse.factoryGetService(ServiceFactoryUse.java:217) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceFactoryUse.getService(ServiceFactoryUse.java:118) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceConsumer$2.getService(ServiceConsumer.java:48) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.getService(ServiceRegistrationImpl.java:547) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.getService(ServiceRegistry.java:534) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.BundleContextImpl.getService(BundleContextImpl.java:660) [org.eclipse.osgi-3.18.0.jar:?]
	at org.apache.felix.scr.impl.manager.SingleRefPair.getServiceObject(SingleRefPair.java:88) [bundleFile:?]
	at org.apache.felix.scr.impl.inject.methods.BindMethod.getServiceObject(BindMethod.java:675) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.DependencyManager.getServiceObject(DependencyManager.java:2612) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.DependencyManager.doInvokeBindMethod(DependencyManager.java:2078) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:2061) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.SingleComponentManager.invokeBindMethod(SingleComponentManager.java:443) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.DependencyManager$MultipleDynamicCustomizer.addedService(DependencyManager.java:336) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.DependencyManager$MultipleDynamicCustomizer.addedService(DependencyManager.java:304) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.ServiceTracker$Tracked.customizerAdded(ServiceTracker.java:1232) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.ServiceTracker$Tracked.customizerAdded(ServiceTracker.java:1152) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.ServiceTracker$AbstractTracked.trackAdding(ServiceTracker.java:959) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.ServiceTracker$AbstractTracked.track(ServiceTracker.java:895) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.ServiceTracker$Tracked.serviceChanged(ServiceTracker.java:1184) [bundleFile:?]
	at org.apache.felix.scr.impl.BundleComponentActivator$ListenerInfo.serviceChanged(BundleComponentActivator.java:116) [bundleFile:?]
	at org.eclipse.osgi.internal.serviceregistry.FilteredServiceListener.serviceChanged(FilteredServiceListener.java:123) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.BundleContextImpl.dispatchEvent(BundleContextImpl.java:961) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:234) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:151) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.publishServiceEventPrivileged(ServiceRegistry.java:937) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.publishServiceEvent(ServiceRegistry.java:874) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.register(ServiceRegistrationImpl.java:141) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.registerService(ServiceRegistry.java:262) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.BundleContextImpl.registerService(BundleContextImpl.java:500) [org.eclipse.osgi-3.18.0.jar:?]
	at org.apache.felix.scr.impl.manager.AbstractComponentManager$3.register(AbstractComponentManager.java:929) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.AbstractComponentManager$3.register(AbstractComponentManager.java:915) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.RegistrationManager.changeRegistration(RegistrationManager.java:133) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.AbstractComponentManager.registerService(AbstractComponentManager.java:984) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.AbstractComponentManager.activateInternal(AbstractComponentManager.java:752) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.AbstractComponentManager.enableInternal(AbstractComponentManager.java:674) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.AbstractComponentManager.enable(AbstractComponentManager.java:437) [bundleFile:?]
	at org.apache.felix.scr.impl.manager.ConfigurableComponentHolder.enableComponents(ConfigurableComponentHolder.java:671) [bundleFile:?]
	at org.apache.felix.scr.impl.BundleComponentActivator.initialEnable(BundleComponentActivator.java:310) [bundleFile:?]
	at org.apache.felix.scr.impl.Activator.loadComponents(Activator.java:593) [bundleFile:?]
	at org.apache.felix.scr.impl.Activator.access$200(Activator.java:74) [bundleFile:?]
	at org.apache.felix.scr.impl.Activator$ScrExtension.start(Activator.java:460) [bundleFile:?]
	at org.apache.felix.scr.impl.AbstractExtender.createExtension(AbstractExtender.java:196) [bundleFile:?]
	at org.apache.felix.scr.impl.AbstractExtender.modifiedBundle(AbstractExtender.java:169) [bundleFile:?]
	at org.apache.felix.scr.impl.AbstractExtender.modifiedBundle(AbstractExtender.java:49) [bundleFile:?]
	at org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:488) [osgi.core-8.0.0.jar:?]
	at org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:420) [osgi.core-8.0.0.jar:?]
	at org.osgi.util.tracker.AbstractTracked.track(AbstractTracked.java:232) [osgi.core-8.0.0.jar:?]
	at org.osgi.util.tracker.BundleTracker$Tracked.bundleChanged(BundleTracker.java:450) [osgi.core-8.0.0.jar:?]
	at org.eclipse.osgi.internal.framework.BundleContextImpl.dispatchEvent(BundleContextImpl.java:949) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:234) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:151) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.EquinoxEventPublisher.publishBundleEventPrivileged(EquinoxEventPublisher.java:229) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.EquinoxEventPublisher.publishBundleEvent(EquinoxEventPublisher.java:138) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.EquinoxEventPublisher.publishBundleEvent(EquinoxEventPublisher.java:130) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.EquinoxContainerAdaptor.publishModuleEvent(EquinoxContainerAdaptor.java:217) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.Module.publishEvent(Module.java:499) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.Module.start(Module.java:486) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel$2.run(ModuleContainer.java:1847) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.internal.framework.EquinoxContainerAdaptor$1$1.execute(EquinoxContainerAdaptor.java:136) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.incStartLevel(ModuleContainer.java:1840) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.incStartLevel(ModuleContainer.java:1783) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.doContainerStartLevel(ModuleContainer.java:1745) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.java:1667) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.container.ModuleContainer$ContainerStartLevel.dispatchEvent(ModuleContainer.java:1) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:234) [org.eclipse.osgi-3.18.0.jar:?]
	at org.eclipse.osgi.framework.eventmgr.EventManager$EventThread.run(EventManager.java:345) [org.eclipse.osgi-3.18.0.jar:?]

This causes a new error for each item:

2024-10-21 09:06:59.260 [WARN ] [jdbc.internal.JdbcPersistenceService] - JDBC::store: Unable to store item
org.openhab.persistence.jdbc.internal.exceptions.JdbcException: Not initialized, unable to find table for item Floor6
	at org.openhab.persistence.jdbc.internal.JdbcMapper.getTable(JdbcMapper.java:359) ~[?:?]
	at org.openhab.persistence.jdbc.internal.JdbcMapper.storeItemValue(JdbcMapper.java:217) ~[?:?]
	at org.openhab.persistence.jdbc.internal.JdbcPersistenceService.internalStore(JdbcPersistenceService.java:174) ~[?:?]
	at org.openhab.persistence.jdbc.internal.JdbcPersistenceService.lambda$1(JdbcPersistenceService.java:146) ~[?:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [?:?]
	at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
	at java.lang.Thread.run(Thread.java:840) [?:?]

[jlaur]

I've tested it for TimescaleDB/PostgreSQL. Everything works fine when tableCaseSensitiveItemNames is on true. But I get the following errors when I set it on false:

Thanks for testing! I see the problem is the way #itemsManageTable# is used here:

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

Line 64 in 52a86eb

sqlCreateItemsTableIfNot = "CREATE TABLE IF NOT EXISTS #itemsManageTable# (itemid SERIAL NOT NULL, #colname# #coltype# NOT NULL, CONSTRAINT #itemsManageTable#_pkey PRIMARY KEY (itemid))";

I'll think about it and provide a fix in the evening.

[jlaur]

I'll think about it and provide a fix in the evening.

This should be fixed now.

[JonathanvdGHU]

I'll think about it and provide a fix in the evening.

This should be fixed now.

It took me some time but I have tested it for TimescaleDB and PostgreSQL. Enabled and disabled tableCaseSensitiveItemNames. Looked in the database and saw the graphs in OpenHAB.

So yeah I also think it's fixed now😊.

[jlaur]

It took me some time but I have tested it for TimescaleDB and PostgreSQL. Enabled and disabled tableCaseSensitiveItemNames. Looked in the database and saw the graphs in OpenHAB.

So yeah I also think it's fixed now😊.

Thanks for testing! Are charts also working now?

I will now test with MySQL.

[JonathanvdGHU]

It took me some time but I have tested it for TimescaleDB and PostgreSQL. Enabled and disabled tableCaseSensitiveItemNames. Looked in the database and saw the graphs in OpenHAB.
So yeah I also think it's fixed now😊.

Thanks for testing! Are charts also working now?

I will now test with MySQL.

Yup, this is the same test as I ran yesterday but it's working now :) (tableCaseSensitiveItemNames: disabled)

[jlaur]

I will now test with MySQL.

Done. I tested persisting values as well as migrations between different naming schemes.

[lsiepel]

Was following this issue, so can conclude my review quickly. LGTM.
@jlaur do you need more test results? PErsonally i only use mysql/mariadb, and you tested that already.

[jlaur]

do you need more test results?

Mostly for other RDBMSs, but I don't know anyone to ask. Only MySQL and PostgreSQL have been tested.

[lsiepel]

ostly for other RDBMSs, but I don't know anyone to ask. Only MySQL and PostgreSQL have been tested

Yeah, other persistence service don't seem to have that many users. I propose to merge and get the changes into the next milestone to get some more testing in the wild. Obviously if some regressions occur, we can fix ti quickly. So the risk is not that high, wdyt?

[jlaur]

Yeah, other persistence service don't seem to have that many users. I propose to merge and get the changes into the next milestone to get some more testing in the wild. Obviously if some regressions occur, we can fix ti quickly. So the risk is not that high, wdyt?

I compared carefully all removed methods to make sure they were exactly the same as the base implementations. In some cases I slightly refactored them before removal, because they had already drifted from the base implementation where they were copied from. Since the base class only calls a method that returns the input string, I do not expect regressions there. So I'd say that the RDBMS with the highest risk of regressions is PostgreSQL, but it's tested, and this part actually also fixes some regressions.

[jlaur]

Since this in essence reverts and reimplements #17587, I'll swap labels.

[JonathanvdGHU]

@jlaur thanks for the bug fix.

[JonathanvdGHU]

@jlaur @lsiepel
One thing I btw noticed while testing/fixing is that the JDBC add-on isn't visible in OpenHAB. I am using it now and it's not visible in the settings and by the total add-ons.

So I used the config file to change the settings of the persistence service each time. It's not related to this fixed bug but maybe related to some things that happened before or is it just me? As you can see in the picture, I didn't had the same problem for the Shelly binding when I worked on it.

I will make an issue for it if just one of you have the same problem😶.

[jlaur]

One thing I btw noticed while testing/fixing is that the JDBC add-on isn't visible in OpenHAB.

I think this is related to the way this bundle is organized into different features:
https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.persistence.jdbc/src/main/feature/feature.xml

This is only an issue when installing the JAR. If you click the "Persistence" sub menu in the left side, you will see all the different sub packages, but they will all look like not installed. I don't know how this can be fixed, but at least it doesn't affect normal users installing through the UI.

[JonathanvdGHU]

One thing I btw noticed while testing/fixing is that the JDBC add-on isn't visible in OpenHAB.

I think this is related to the way this bundle is organized into different features: https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.persistence.jdbc/src/main/feature/feature.xml

This is only an issue when installing the JAR. If you click the "Persistence" sub menu in the left side, you will see all the different sub packages, but they will all look like not installed. I don't know how this can be fixed, but at least it doesn't affect normal users installing through the UI.

Good to know. Thanks. Only weird that it isn't the case for the Shelly binding but it has it reasons I guess.