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.

Sign up for GitHub

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

[jdbc] Fix tableCaseSensitiveItemNames for PostgreSQL/TimescaleDB #17587

Merged
merged 12 commits into from
Oct 19, 2024
Merged

[jdbc] Fix tableCaseSensitiveItemNames for PostgreSQL/TimescaleDB #17587

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cee4012
small changes for uppercase
JonathanvdGHU Oct 18, 2024
563add6
table bug fixed
JonathanvdGHU Oct 18, 2024
8447a0d
re-added deleted changed from rebase
JonathanvdGHU Oct 18, 2024
160f720
removed not related code to the bug
JonathanvdGHU Oct 18, 2024
45cc154
implemented feedback
JonathanvdGHU Oct 19, 2024
53be80c
changes also implied on itemsmanagetable
JonathanvdGHU Oct 19, 2024
2874b51
reverted notes
JonathanvdGHU Oct 19, 2024
0c147d4
reverted missed notes
JonathanvdGHU Oct 19, 2024
83e6209
removed unnecessary methods
JonathanvdGHU Oct 19, 2024
48b0a73
table bug fixed
JonathanvdGHU Oct 18, 2024
1da3b7b
Merge branch 'timescaledb-table-bug' of https://github.com/Jonathanvd…
JonathanvdGHU Oct 19, 2024
7d1b4ab
Remove excess line
jlaur Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions ...stence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/db/JdbcPostgresqlDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,25 +60,28 @@ private void initSqlQueries() {
logger.debug("JDBC::initSqlQueries: '{}'", this.getClass().getSimpleName());
// System Information Functions: https://www.postgresql.org/docs/9.2/static/functions-info.html
sqlGetDB = "SELECT CURRENT_DATABASE()";
sqlIfTableExists = "SELECT * FROM PG_TABLES WHERE TABLENAME='#searchTable#'";
sqlCreateItemsTableIfNot = "CREATE TABLE IF NOT EXISTS #itemsManageTable# (itemid SERIAL NOT NULL, #colname# #coltype# NOT NULL, CONSTRAINT #itemsManageTable#_pkey PRIMARY KEY (itemid))";
sqlCreateNewEntryInItemsTable = "INSERT INTO items (itemname) SELECT itemname FROM #itemsManageTable# UNION VALUES ('#itemname#') EXCEPT SELECT itemname FROM items";
sqlIfTableExists = "SELECT * FROM PG_TABLES WHERE TABLENAME='\"#searchTable#\"'";
sqlDropTable = "DROP TABLE \"#tableName#\"";
sqlCreateItemsTableIfNot = "CREATE TABLE IF NOT EXISTS \"#itemsManageTable#\" (itemid SERIAL NOT NULL, #colname# #coltype# NOT NULL, CONSTRAINT #itemsManageTable#_pkey PRIMARY KEY (itemid))";
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#'\
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#')\
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#\"')\
Comment on lines +76 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonathanvdGHU - I think the quotes here were a mistake, since the table names are not used for referencing the tables directly, but rather being used to query the information schema. Do you agree? See also #17597.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what you mean, but to be honest I am not sure how postgreSQL reacts when you remove them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends how it internally stores the table names. Would it possibly for you to run a query so we can see?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try it when I have the time.

Copy link
Contributor Author

@JonathanvdGHU JonathanvdGHU Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same case here. (it's PostgreSQL btw, I know a little bit confusing)
image

Copy link
Contributor

@jlaur jlaur Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try:

SELECT table_name
FROM information_schema.tables

I would expect it to return e.g. TotalUsage rather than "TotalUsage".

Or to be specific as in your query (single quotes were missing):

SELECT *
FROM information_schema.tables
WHERE table_name = 'TotalUsage'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
2. (still postgreSQL)
image

Copy link
Contributor

@jlaur jlaur Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks. So indeed the double-quotes should not be included in those queries, thus already fixed as part of #17597.

""";
// 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#) )";
sqlAlterTableColumn = "ALTER TABLE #tableName# ALTER COLUMN #columnName# TYPE #columnType#";
sqlInsertItemValue = "INSERT INTO \"#tableName#\" (TIME, VALUE) VALUES( #tablePrimaryValue#, CAST( ? as #dbType#) )";
sqlCreateItemTable = "CREATE TABLE IF NOT EXISTS \"#tableName#\" (time #tablePrimaryKey# NOT NULL, value #dbType#, PRIMARY KEY(time))";
sqlAlterTableColumn = "ALTER TABLE \"#tableName#\" ALTER COLUMN #columnName# TYPE #columnType#";
sqlGetRowCount = "SELECT COUNT(*) FROM \"#tableName#\"";
}

@Override
Expand All @@ -92,7 +95,7 @@ public void initAfterFirstDbConnection() {
if (dbMeta.isDbVersionGreater(9, 4)) {
logger.debug("JDBC::initAfterFirstDbConnection: Values with the same time will be upserted (Pg >= 9.5)");
sqlInsertItemValue = """
INSERT INTO #tableName# (TIME, VALUE) VALUES( #tablePrimaryValue#, CAST( ? as #dbType#) )\
INSERT INTO \"#tableName#\" (TIME, VALUE) VALUES( #tablePrimaryValue#, CAST( ? as #dbType#) )\
ON CONFLICT (TIME) DO UPDATE SET VALUE=EXCLUDED.VALUE\
""";
}
Expand Down Expand Up @@ -202,6 +205,7 @@ public List<Column> doGetTableColumns(ItemsVO vo) throws JdbcSQLException {
/*
* 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 {
Expand All @@ -213,7 +217,7 @@ public void doAlterTableColumn(String tableName, String columnName, String colum
Yank.execute(sql, null);
if (!nullable) {
String sql2 = StringUtilsExt.replaceArrayMerge(
"ALTER TABLE #tableName# ALTER COLUMN #columnName# SET NOT NULL",
"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);
Expand Down
21 changes: 20 additions & 1 deletion ...tence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/db/JdbcTimescaledbDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
*/
package org.openhab.persistence.jdbc.internal.db;

import java.util.List;
import java.util.Properties;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.knowm.yank.Yank;
import org.knowm.yank.exceptions.YankSQLException;
import org.openhab.persistence.jdbc.internal.dto.ItemVO;
import org.openhab.persistence.jdbc.internal.dto.ItemsVO;
import org.openhab.persistence.jdbc.internal.exceptions.JdbcSQLException;
import org.openhab.persistence.jdbc.internal.utils.StringUtilsExt;
import org.slf4j.Logger;
Expand All @@ -34,7 +36,8 @@
public class JdbcTimescaledbDAO extends JdbcPostgresqlDAO {
private final Logger logger = LoggerFactory.getLogger(JdbcTimescaledbDAO.class);

private final String sqlCreateHypertable = "SELECT created from create_hypertable('#tableName#', 'time')";
private final String sqlCreateHypertable = "SELECT created FROM create_hypertable('\"#tableName#\"', 'time')";
private final String sqlGetItemTables = "SELECT hypertable_name as table_name FROM timescaledb_information.hypertables WHERE hypertable_name != '\"#itemsManageTable#\"'";

@Override
public Properties getConnectionProperties() {
Expand All @@ -46,6 +49,10 @@ public Properties getConnectionProperties() {
return properties;
}

/*************
* ITEM DAOs *
*************/

@Override
public void doCreateItemTable(ItemVO vo) throws JdbcSQLException {
super.doCreateItemTable(vo);
Expand All @@ -58,4 +65,16 @@ public void doCreateItemTable(ItemVO vo) throws JdbcSQLException {
throw new JdbcSQLException(e);
}
}

@Override
public List<ItemsVO> doGetItemTables(ItemsVO vo) throws JdbcSQLException {
String sql = StringUtilsExt.replaceArrayMerge(sqlGetItemTables, new String[] { "#itemsManageTable#" },
new String[] { vo.getItemsManageTable() });
this.logger.debug("JDBC::doGetItemTables sql={}", sql);
try {
return Yank.queryBeanList(sql, ItemsVO.class, null);
} catch (YankSQLException e) {
throw new JdbcSQLException(e);
}
}
}