NH-3889 - Fixed weird extra joins in subqueries

[PleasantD]

See https://nhibernate.jira.com/browse/NH-3889

[hazzik]

No commented code, please

[hazzik]

Please fix new tests in the Firbird, and otherwise good to go.

[PleasantD]

Fixed up.
Does the Firebird provider not escape column or table names? I think this is the second time I've had conflicts with reserved keywords in Firebird.

[hazzik]

@PleasantD not sure about Firebird

[hazzik]

LGTM

[PleasantD]

Rebased on master

[PleasantD]

Can this be included in 4.1.0?

[oskarb]

Thanks for rebase. I think I've noticed this phenomenon too. Can you check if Hibernate have made a similar change or otherwise differ from what we have in this code?

[PleasantD]

Hibernate seems to grammar similar to ours before my change.
https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/antlr/hql-sql.g

selectExprList is identical to NH before the change
query! is similar to the NH unionedQuery! before the change, although the NH grammar seems to cover more syntax possibilities.

So, no, it doesn't look like they have made any similar fix.

[oskarb]

The test cases fail on SQL Server CE. We need some resolution for that before merging.

[PleasantD]

I rolled back the grammar change and ran the tests.
The same 4 tests were failing before the the change, with the same error:

[SQL: select job0_.Id as col_0_0_, (select cast(sum(timerecord1_.Hours) as NUMERIC(19,5)) from TimeRecord timerecord1_ left outer join Project project2_ on timerecord1_.ProjectId=project2_.Id inner join TimeSetting timesettin3_ on timerecord1_.SettingId=timesettin3_.Id, TimeInclude timeinclud4_ where timesettin3_.IncludeId=timeinclud4_.Id and coalesce(timerecord1_.ActualJobId, project2_.JobId)=job0_.Id and timesettin3_.IncludeId=@p0) as col_1_0_, job0_.Id as Id1_0_, job0_.Name as Name2_0_ from Job job0_]
  ----> System.Data.SqlServerCe.SqlCeException : There was an error parsing the query. [ Token line number = 1,Token line offset = 31,Token in error = select ]

The issue isn't with the grammar change, but that SQL Server CE does not support subqueries that return a scalar result. The four failing tests all generate a subquery with a SUM result in the SELECT clause which just won't parse for SQL Server CE.

With that in mind, what is the policy for ignoring tests that fail on a single dialect?

[hazzik]

Hi @PleasantD, can you please enable Allow edits from maintainers feature?

[PleasantD]

Done

[hazzik]

Thanks, @PleasantD. Merged