Skip to content

Conversation

niekschoemaker
Copy link
Contributor

Shamelessly copied from the postgres sql provider, that's why I added the copyright notice above it, no need to reinvent the wheel as they already solved it.

Implement VisitCrossApply and VisitOuterApply methods to translate CROSS APPLY and OUTER APPLY constructs for Firebird using lateral joins. Adapted logic from Npgsql EF Core provider.

Related to issue #1165

@niekschoemaker niekschoemaker force-pushed the add-support-for-lateral-joins branch from bb97998 to b5d272a Compare September 26, 2025 14:10
Implement `VisitCrossApply` and `VisitOuterApply` methods to translate CROSS APPLY and OUTER APPLY constructs for Firebird using lateral joins. Adapted logic from Npgsql EF Core provider.
@niekschoemaker niekschoemaker force-pushed the add-support-for-lateral-joins branch from b5d272a to d7af605 Compare September 26, 2025 14:14
@cincuranet
Copy link
Member

There's a lot of tests that result in CROSS APPLY/OUTER APPLY that are ignored now, these need to be enabled. You can start by searching for NotSupportedOnFirebirdFact/NotSupportedOnFirebirdTheory.

@niekschoemaker
Copy link
Contributor Author

There's a lot of tests that result in CROSS APPLY/OUTER APPLY that are ignored now, these need to be enabled. You can start by searching for NotSupportedOnFirebirdFact/NotSupportedOnFirebirdTheory.

Just making sure, I'm going through all disabled tests at the moment, and if I spot things which are marked as unsupported by efcore, but which are supported now, should those be enabled too?

@cincuranet
Copy link
Member

if I spot things which are marked as unsupported by efcore, but which are supported now, should those be enabled too?

Yes. But create separate PR.

@niekschoemaker niekschoemaker force-pushed the add-support-for-lateral-joins branch from 4b38935 to 05b6a29 Compare September 30, 2025 11:12
@niekschoemaker niekschoemaker marked this pull request as draft September 30, 2025 11:34
@niekschoemaker niekschoemaker force-pushed the add-support-for-lateral-joins branch from 05b6a29 to c1acf51 Compare September 30, 2025 11:39
@niekschoemaker niekschoemaker marked this pull request as ready for review September 30, 2025 13:12
@niekschoemaker niekschoemaker marked this pull request as draft September 30, 2025 13:33
@niekschoemaker niekschoemaker force-pushed the add-support-for-lateral-joins branch 4 times, most recently from a1b7428 to 02a6dca Compare September 30, 2025 14:13
@niekschoemaker
Copy link
Contributor Author

Alright, sorry about the mess with force pushes, I completely overlooked the Firebird 3 and 4 support so naively tested on firebird 5 and enabled way too many tests.
Commits look clean and on the fork the tests passed so should be good now!

@niekschoemaker niekschoemaker marked this pull request as ready for review September 30, 2025 14:46
@niekschoemaker niekschoemaker force-pushed the add-support-for-lateral-joins branch from 02a6dca to 8799936 Compare October 1, 2025 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants