Fix getTableNames for cockroachdb #3752

rafiss · 2023-09-24T14:33:42Z

The query to fetch table names was incorrectly including internal tables that are part of the crdb_internal schema.

Previously, running tests would fail with an error:

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 26.134 s <<< FAILURE! - in org.flowable.engine.test.api.v6.Flowable6Test
[ERROR] org.flowable.engine.test.api.v6.Flowable6Test.testLongServiceTaskLoop  Time elapsed: 26.129 s  <<< ERROR!
org.flowable.common.engine.api.FlowableException: couldn't get table counts
        at org.flowable.common.engine.impl.persistence.entity.TableDataManagerImpl.getTableCount(TableDataManagerImpl.java:59)
        at org.flowable.common.engine.impl.test.EnsureCleanDbUtils.lambda$assertAndEnsureCleanDb$0(EnsureCleanDbUtils.java:55)
...
Caused by: org.apache.ibatis.exceptions.PersistenceException:
### Error querying database.  Cause: org.postgresql.util.PSQLException: ERROR: relation "active_range_feeds" does not exist
### The error may exist in org/flowable/common/db/mapping/entity/TableData.xml
### The error may involve org.flowable.common.engine.impl.TablePageMap.selectTableCount-Inline
### The error occurred while setting parameters
### SQL: select count(*) from ACTIVE_RANGE_FEEDS
### Cause: org.postgresql.util.PSQLException: ERROR: relation "active_range_feeds" does not exist
        at org.apache.ibatis.exceptions.ExceptionFactory.wrapException(ExceptionFactory.java:30)
        at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:153)
        at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:145)
        at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:140)
        at org.apache.ibatis.session.defaults.DefaultSqlSession.selectOne(DefaultSqlSession.java:76)
        at org.flowable.common.engine.impl.db.DbSqlSession.selectOne(DbSqlSession.java:286)
        at org.flowable.common.engine.impl.persistence.entity.TableDataManagerImpl.getTableCount(TableDataManagerImpl.java:138)
        at org.flowable.common.engine.impl.persistence.entity.TableDataManagerImpl.getTableCount(TableDataManagerImpl.java:55)
        ... 76 more
Caused by: org.postgresql.util.PSQLException: ERROR: relation "active_range_feeds" does not exist
        at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2676)
        at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2366)
        at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:356)
        at org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:496)
        at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:413)
        at org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:190)
        at org.postgresql.jdbc.PgPreparedStatement.execute(PgPreparedStatement.java:177)
        at com.zaxxer.hikari.pool.ProxyPreparedStatement.execute(ProxyPreparedStatement.java:44)
        at com.zaxxer.hikari.pool.HikariProxyPreparedStatement.execute(HikariProxyPreparedStatement.java)
        at jdk.internal.reflect.GeneratedMethodAccessor27.invoke(Unknown Source)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.apache.ibatis.logging.jdbc.PreparedStatementLogger.invoke(PreparedStatementLogger.java:59)
        at jdk.proxy3/jdk.proxy3.$Proxy22.execute(Unknown Source)
        at org.apache.ibatis.executor.statement.PreparedStatementHandler.query(PreparedStatementHandler.java:64)
        at org.apache.ibatis.executor.statement.RoutingStatementHandler.query(RoutingStatementHandler.java:79)
        at org.apache.ibatis.executor.SimpleExecutor.doQuery(SimpleExecutor.java:63)
        at org.apache.ibatis.executor.BaseExecutor.queryFromDatabase(BaseExecutor.java:325)
        at org.apache.ibatis.executor.BaseExecutor.query(BaseExecutor.java:156)
        at org.apache.ibatis.executor.CachingExecutor.query(CachingExecutor.java:109)
        at org.apache.ibatis.executor.CachingExecutor.query(CachingExecutor.java:89)
        at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:151)
        ... 82 more

Check List:

Unit tests: NA (tests were previously broken with cockroachdb, and now they work)
Documentation: NA

The query to fetch table names was incorrectly including internal tables that are part of the crdb_internal schema.

CockroachDB has auto-commit, so forcing a commit would result in an error.

filiphr · 2023-09-25T07:00:15Z

Thanks @rafiss for the PR.

I have some questions for the changes.

The table name filter that we are using is ACT_% and FLW_%. Why is the call databaseMetaData.getTables(catalog, schema, tableNameFilter, DbSqlSession.JDBC_METADATA_TABLE_TYPES) returning active_range_feeds?

We are also getting the DatabaseMetaData from the current connection. Shouldn't that return data that it has access to only? i.e. from the database configured schema?

Is it possible that this is some kind of a bug in cockroachdb?

Thanks for the improvements with -- force-commit

rafiss · 2023-09-25T14:08:32Z

Thanks for taking a look!

The table name filter that we are using is ACT_% and FLW_%. Why is the call databaseMetaData.getTables(catalog, schema, tableNameFilter, DbSqlSession.JDBC_METADATA_TABLE_TYPES) returning active_range_feeds?

That's because when using LIKE in both PostgreSQL and CockroachDB, _ matches any single character. This is described here: https://www.postgresql.org/docs/current/functions-matching.html#FUNCTIONS-LIKE

We are also getting the DatabaseMetaData from the current connection. Shouldn't that return data that it has access to only? i.e. from the database configured schema?

I used Wireshark to capture the query that Flowable was sending. It was this:

SELECT
	NULL AS table_cat,
	n.nspname AS table_schem,
	c.relname AS table_name,
	CASE n.nspname ~ '^pg_'
	OR n.nspname = 'information_schema'
	WHEN true
	THEN CASE
	WHEN n.nspname = 'pg_catalog'
	OR n.nspname = 'information_schema'
	THEN CASE c.relkind
	WHEN 'r' THEN 'SYSTEM TABLE'
	WHEN 'v' THEN 'SYSTEM VIEW'
	WHEN 'i' THEN 'SYSTEM INDEX'
	ELSE NULL
	END
	WHEN n.nspname = 'pg_toast'
	THEN CASE c.relkind
	WHEN 'r' THEN 'SYSTEM TOAST TABLE'
	WHEN 'i' THEN 'SYSTEM TOAST INDEX'
	ELSE NULL
	END
	ELSE CASE c.relkind
	WHEN 'r' THEN 'TEMPORARY TABLE'
	WHEN 'p' THEN 'TEMPORARY TABLE'
	WHEN 'i' THEN 'TEMPORARY INDEX'
	WHEN 'S' THEN 'TEMPORARY SEQUENCE'
	WHEN 'v' THEN 'TEMPORARY VIEW'
	ELSE NULL
	END
	END
	WHEN false
	THEN CASE c.relkind
	WHEN 'r' THEN 'TABLE'
	WHEN 'p' THEN 'PARTITIONED TABLE'
	WHEN 'i' THEN 'INDEX'
	WHEN 'P' THEN 'PARTITIONED INDEX'
	WHEN 'S' THEN 'SEQUENCE'
	WHEN 'v' THEN 'VIEW'
	WHEN 'c' THEN 'TYPE'
	WHEN 'f' THEN 'FOREIGN TABLE'
	WHEN 'm' THEN 'MATERIALIZED VIEW'
	ELSE NULL
	END
	ELSE NULL
	END
		AS table_type,
	d.description AS remarks,
	'' AS type_cat,
	'' AS type_schem,
	'' AS type_name,
	'' AS self_referencing_col_name,
	'' AS ref_generation
FROM
	pg_catalog.pg_namespace AS n,
	pg_catalog.pg_class AS c
	LEFT JOIN pg_catalog.pg_description AS d ON
			(
				c.oid = d.objoid
				AND d.objsubid = 0
				AND d.classoid = 'pg_class'::REGCLASS
			)
WHERE
	c.relnamespace = n.oid
	AND c.relname LIKE 'act_%'
	AND (
			false
			OR (
					c.relkind = 'r'
					AND n.nspname !~ '^pg_'
					AND n.nspname != 'information_schema'
				)
		)
ORDER BY
	table_type, table_schem, table_name;

From a read of the PGJDBC getTables method (here), that means that the schemaPattern argument was null, so Flowable did not pass in a value when it made the call here:

flowable-engine/modules/flowable-engine-common/src/main/java/org/flowable/common/engine/impl/persistence/entity/TableDataManagerImpl.java

Lines 100 to 111 in 09fe1bd

    
           protected List<String> getTableNames(DatabaseMetaData databaseMetaData, String catalog, String schema, String tableNameFilter) throws SQLException { 
        
               List<String> tableNames = new ArrayList<>(); 
        
               try (ResultSet tables = databaseMetaData.getTables(catalog, schema, tableNameFilter, DbSqlSession.JDBC_METADATA_TABLE_TYPES)) { 
        
                   while (tables.next()) { 
        
                       String tableName = tables.getString("TABLE_NAME"); 
        
                       tableName = tableName.toUpperCase(Locale.ROOT); 
        
                       tableNames.add(tableName); 
        
                       LOGGER.debug("retrieved flowable table name {}", tableName); 
        
                   } 
        
               } 
        
               return tableNames;

filiphr · 2023-09-25T15:19:07Z

That's because when using LIKE in both PostgreSQL and CockroachDB, _ matches any single character. This is described here: https://www.postgresql.org/docs/current/functions-matching.html#FUNCTIONS-LIKE

Thanks for pointing that out. Seems like a bug in that case. In that case a solution like the one for Oracle would work here I guess. e.g. if we do

tableNameFilter = databaseTablePrefix + flowableTablePrefix.toLowerCase(Locale.ROOT) + databaseMetaData.getSearchStringEscape() +"_%";

I'll talk with the rest of the team and we'll try to merge this as soon as possible.

From a read of the PGJDBC getTables method (here), that means that the schemaPattern argument was null, so Flowable did not pass in a value when it made the call here:

I made a wrong assumption. I was thinking that the user does not have access to the schema. However, the problem is that the user does have access, so we will get it when querying from the database metatada. The problem happens when we try to get the count.

rafiss · 2023-11-18T08:09:36Z

Hi! Just wanted to check if that fix you mentioned above is still going in?

Thanks for pointing that out. Seems like a bug in that case. In that case a solution like the one for Oracle would work here I guess. e.g. if we do
tableNameFilter = databaseTablePrefix + flowableTablePrefix.toLowerCase(Locale.ROOT) + databaseMetaData.getSearchStringEscape() +"_%";
I'll talk with the rest of the team and we'll try to merge this as soon as possible.

filiphr · 2023-11-20T13:38:00Z

Thanks for the ping @rafiss, sorry that it took a bit longer.

I've pushed a fix for this in 3eca3b9. I believe that this now solves the problem then you are having. Can you perhaps try with what is on main?

Apart from the getTableNames fix, I see that in this PR you've also removed some -- force-commit from our SQL scripts for CockroachDB. I talked to @jbarrez, since he was the one that added those back when we were adding the support. Seems like they were needed at that time. Has something changed on CockroachDB on that side?

This was referenced Sep 24, 2023

roachtest: flowable failed cockroachdb/cockroach#111032

Closed

roachtest: flowable failed cockroachdb/cockroach#111102

Closed

roachtest: flowable failed cockroachdb/cockroach#111029

Closed

roachtest: flowable failed cockroachdb/cockroach#111027

Closed

rafiss added 2 commits September 24, 2023 10:59

Fix getTableNames for cockroachdb

73fba80

The query to fetch table names was incorrectly including internal tables that are part of the crdb_internal schema.

Remove force commit from cockroachdb setup

9ce12ce

CockroachDB has auto-commit, so forcing a commit would result in an error.

rafiss force-pushed the crdb-table-name branch from 85aadff to 9ce12ce Compare September 24, 2023 15:00

rafiss closed this Jan 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix getTableNames for cockroachdb #3752

Fix getTableNames for cockroachdb #3752

rafiss commented Sep 24, 2023

filiphr commented Sep 25, 2023

rafiss commented Sep 25, 2023 •

edited

Loading

filiphr commented Sep 25, 2023

rafiss commented Nov 18, 2023

filiphr commented Nov 20, 2023

Fix getTableNames for cockroachdb #3752

Fix getTableNames for cockroachdb #3752

Conversation

rafiss commented Sep 24, 2023

Check List:

filiphr commented Sep 25, 2023

rafiss commented Sep 25, 2023 • edited Loading

filiphr commented Sep 25, 2023

rafiss commented Nov 18, 2023

filiphr commented Nov 20, 2023

rafiss commented Sep 25, 2023 •

edited

Loading