For update with outer join fails with PostgreSQL

[Doan-Van-Tuan]

When building SQL in OuterJoinLoader, Dialect.SupportsOuterJoinForUpdate is not being respected. This causes an exception with Postgres because Postgres will not accept blank lock hint with query containing an outer join such as this query:

SELECT entity0_.Id as id1_0_0_, entity0_1_.Name as name2_1_0_
  FROM Entities entity0_
  left outer join EntityName entity0_1_ on entity0_.Id=entity0_1_.EntityId
  WHERE entity0_.Id=? for update

If we change SqlSelectBuilder to use lock hint for update of entity0_ instead of for update, the query can be executed.

I will create a pull request in next few days.

[Doan-Van-Tuan]

I have created a pull-request #1568
Please have a look.

[fredericDelaporte]

I have allowed myself to rewrite a bit this ticket title and description for clarity.

That said, SupportsOuterJoinForUpdate default value is true, and PostgreSQL does not override it to false. Reading from this discussion about this for update trouble but raised on PostgreSQL side by a NHibernate contributor, it seems to me it should just override it to false.

The addition of of alias clause seems to be a workaround for partially locking the row, but which will still miss locking rows from the left join when there are any. And this clause, taking table aliases, looks quite specific to PostgreSQL. It seems other databases may take column names instead (read from Dialect.GetForUpdateString(string aliases) comment) . So it appears to me the for update should simply be disabled for PostgreSQL when there is an outer join.

[fredericDelaporte]

Checking the code a bit more, it looks like that is the comment which is wrong. The already existing code use it with table aliases.

So in such case, why not locking only the from table as you propose, it is still better than not locking anything.

[fredericDelaporte]

I have misread the code. Indeed, the of clause can take both a columns list or a table aliases list, according to bool Dialect.ForUpdateOfColumns. So my first comment was valid, always supplying a table alias is specific to PostgreSQL.