datanucleus / datanucleus-rdbms

DataNucleus support for persistence to RDBMS Datastores
30 stars 66 forks source link

`UpdateStmtAllowTableAliasInSet` is not honored for update statements involving `DelegatedExpression`s #474

Closed nscuro closed 2 months ago

nscuro commented 1 year ago

Bug Report

The UpdateStmtAllowTableAliasInSet option of data store adapters (e.g. PostgreSQLAdapter) is not honored for DelegatedExpressions like EnumExpression. This causes compilation of invalid SQL queries for databases like PostgreSQL.


The following JDO Bulk Update query:

UPDATE org.acme.WorkflowState
SET status = :status, updatedAt = :updatedAt
WHERE status == :pendingStatus && startedAt != null && updatedAt < :threshold

fails to be executed against a PostgreSQL database, due to the following exception:

javax.jdo.JDOException: Exception thrown when executing query : UPDATE "WORKFLOW_STATE" "A0" SET "A0"."STATUS" = ?,"UPDATED_AT" = ? WHERE "A0"."STATUS" = ? AND "A0"."STARTED_AT" IS NOT NULL AND "A0"."UPDATED_AT" < ?
...
org.postgresql.util.PSQLException: ERROR: column "A0" of relation "WORKFLOW_STATE" does not exist

In this case status is an enum (EnumExpression at query compile time).

PostgreSQL does not allow for table names in SET expressions, and DataNucleus correctly handles this, except for SQLExpressions extending DelegatedExpression.

I have traced the issue down to QueryToSQLMapper#compileUpdate, where setOmitTableFromString is called for all ColumnExpressions of a given SQLExpression:

https://github.com/datanucleus/datanucleus-rdbms/blob/fb0bcf27f9c0e8ffc558e1737601254f9570d044/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java#L1084-L1092

Note that getSubExpression does not consider the delegate expression of DelegatedExpressions. So setOmitTableFromString is never applied to sub expressions of delegate.

Towards the end of compileUpdate, the following is executed:

https://github.com/datanucleus/datanucleus-rdbms/blob/fb0bcf27f9c0e8ffc558e1737601254f9570d044/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java#L1142

eq is directly delegated to the DelegatedExpression's delegate:

https://github.com/datanucleus/datanucleus-rdbms/blob/fb0bcf27f9c0e8ffc558e1737601254f9570d044/src/main/java/org/datanucleus/store/rdbms/sql/expression/DelegatedExpression.java#L40-L47

Due to this, the final BooleanExpression is using ColumnExpressions on which setOmitTableFromString has never been called. Thus, the error described above is happening.


Should DelegatedExpression not also delegate calls to getNumberOfSubExpressions and getSubExpression to its delegate? Alternatively, should setOmitTableFromString be called for all sub expressions of the parent and delegate expression?

andyjefferson commented 1 year ago

Why not present a valid testcase that demonstrates something? As per https://www.datanucleus.org/documentation/problem_reporting.html#jdo

andyjefferson commented 2 months ago

getNumberOfSubExpressions, getSubExpression, isParameter, invoke were added as delegated back in this commit https://github.com/datanucleus/datanucleus-rdbms/commit/dddee142870e086dae5647845d65a93bb5c2b209

Since no testcase is provided for this issue then nothing to say about whether it "fixes" it or otherwise. Closing. If the issue is not fixed then kindly provide a testcase as per the problem reporting guide.