Emit a table-valued function without LATERAL. Fix #1277 - #1279
Open
azabluda wants to merge 2 commits into
Open
Conversation
The fourteen QF_*Correlated*/QF_*Apply* tests were skipped as "Not supported on Firebird". Twelve of them need nothing Firebird-specific at all, so their overrides go away and the base tests run on every version; the two that reach the store as a lateral derived table keep a Firebird 4 gate. The fixture was missing AddValues, GetCustomerOrderCountByYear and GetCustomerOrderCountByYearOnlyFrom2000, and nothing noticed because every test that uses them was skipped. They are ported from EF Core's own UdfDbFunctionSqlServerTests fixture, in Firebird form: the two table-valued ones become selectable procedures, as the two already here are, and year(...) becomes extract(year from ...). EF Core names a queryable function after its method unless told otherwise, and GetCustomerOrderCountByYearOnlyFrom2000 is 39 characters, which Firebird 3 will not create. It is mapped to a 31-character name instead, the way GetCustWithMostOrdersAfterDate already is here. This commit is red: the twelve tests fail on a malformed statement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
VisitCrossApply and VisitOuterApply put LATERAL in front of whatever EF Core
hands them. In Firebird's grammar LATERAL is a prefix on a derived table only:
<table-primary> ::=
<table-or-query-name> [[AS] correlation-name]
| [LATERAL] <derived-table> [<correlation-or-recognition>]
| <parenthesized-joined-table>
<table-or-query-name> ::=
table-name | query-name | [package-name.]procedure-name [(<procedure-args>)]
A selectable procedure is the other alternative, and its arguments may already
reference streams declared earlier in the FROM clause. So a correlated queryable
function needs no LATERAL - and cannot have one, which is why it did not parse.
JOIN LATERAL "GetCustomerOrderCountByYear"("c"."Id") AS "g" ON TRUE
-- Dynamic SQL Error, SQL error code = -104, Token unknown
JOIN "GetCustomerOrderCountByYear"("c"."Id") AS "g" ON TRUE
-- works, on Firebird 3 as well
The two visitors now share one GenerateApplySource, which emits the call as it
is emitted anywhere else and keeps LATERAL for the sources that need it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1277. Replaces #1278, which fixed the same defect by wrapping the call in a derived table. @mrotteveel pointed out on the issue that joins with selectable procedures are implicitly lateral, so the
LATERALthat wrapping worked around does not need to be there at all.The defect
VisitCrossApplyandVisitOuterApplyputLATERALin front of whatever EF Core hands them. In Firebird's grammarLATERALis a prefix on a derived table only:A selectable procedure is the other alternative, and its
<procedure-args>may already reference streams declared earlier in theFROMclause. So a correlated queryable function cannot takeLATERAL, and does not need one:The two visitors now share one
GenerateApplySource, which emits the call the way it is emitted everywhere else and keepsLATERALfor the sources that really are derived tables. The generator loses more lines than it gains.Two commits, and the first is red on purpose
Passed 84, Failed 12, Skipped 10Passed 96, Failed 0, Skipped 10Split so the fix is measured against a failing suite rather than asserted.
What this changes for Firebird 3
Twelve of the fourteen
QF_*Correlated*/QF_*Apply*tests were skipped as "Not supported on Firebird". They now run on every supported version, and need no Firebird-specific override at all, so those overrides are deleted rather than rewritten.Two keep a Firebird 4 check:
QF_Select_Correlated_Subquery_In_AnonymousandQF_Correlated_Func_Call_With_Navigation. EF Core wraps the call in aSELECTof its own for those, so the source of the join is a derived table that reads an outer column, and it reaches the store as a genuine lateral derived table:The check is the same early return that
NavigationsCollectionFbTestandComplexTypeQueryFbTestalready use.The fixture was missing three objects
AddValues,GetCustomerOrderCountByYearandGetCustomerOrderCountByYearOnlyFrom2000were never created, and nothing noticed, because every test that uses them was skipped. They are ported from the definitions in EF Core's ownUdfDbFunctionSqlServerTestsfixture, in Firebird form: the two table-valued ones become selectable procedures, as the two already in this fixture are, andyear(...)becomesextract(year from ...).EF Core names a queryable function after its method, and
GetCustomerOrderCountByYearOnlyFrom2000is 39 characters, which Firebird 3 will not create. It is mapped to a 31-character name, next to the twoGetCustWithMostOrdersAfterDatemappings that are already there for exactly the same reason.What was run
UdfDbFunctionFbTestslocally, against both commits:The full
FirebirdSql.EntityFrameworkCore.Firebird.FunctionalTestssuite on the second commit:Passed 14167, Failed 0, Skipped 1113, Total 15280, identical on 3.0.11 and on 5.0.0.FB30, FB40 and FB50 are green across every suite in my fork: https://github.com/azabluda/NETProvider/actions/runs/34227354250
Both SQL forms were also run directly against Firebird 3.0.11 by hand, including the shapes the implicit form has to survive: inner and left join, a chained call fed from another procedure stream, a call inside a derived table, expression arguments, and a derived table as the outer source.
AI usage
Per CONTRIBUTING: this change was written with Claude Code. I reviewed it, ran it and stand behind it. Both commits carry a
Co-Authored-Bytrailer.