Work on inlined primitive collections (VALUES) #36159
Conversation
roji
dismissed
cincuranet’s stale review
Brought up to date and made various changes, re-review needed.
There was a problem hiding this comment.
@cincuranet I've brought this back from the dead - rebased on latest main and uncovered a couple of extra bugs which I fixed. Can you please give this another review?
| var intTypeMapping = (IntTypeMapping?)Dependencies.TypeMappingSource.FindMapping(typeof(int)); | ||
| Check.DebugAssert(intTypeMapping is not null); | ||
| var valuesOrderingCounter = 1; | ||
| var valuesOrderingCounter = 0; |
There was a problem hiding this comment.
@cincuranet I did this tiny change to align our ordering counter to start at zero only because that's what we do in translation for regular ValuesExpression (where there's no EF.Constant() etc.). Of course the actual values here don't matter - we only order by them (we can also go the other way and start with 1 ;)).
There was a problem hiding this comment.
I started with 1 because in SQL stuff is usually indexed from 1. But I don't feel strongly about it.
test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
Show resolved
Hide resolved
* Refactor and generalize the pruning of the VALUES _ord column, moving it from RelationalTypeMappingPostprocessor to SqlPruner. * Ensure that inlined parameterized collections (EF.Constant) preserved ordering via _ord and specifies the type mapping. * Properly visit JsonScalarExpression in SqlNullabilityProcessor. * Add CAST() to VALUES generated in SqlNullabilityProcessor because of EF.Constant(). Fixes dotnet#36158 Fixes dotnet#36462 Fixes dotnet#36463
| var intTypeMapping = (IntTypeMapping?)Dependencies.TypeMappingSource.FindMapping(typeof(int)); | ||
| Check.DebugAssert(intTypeMapping is not null); | ||
| var valuesOrderingCounter = 1; | ||
| var valuesOrderingCounter = 0; |
There was a problem hiding this comment.
I started with 1 because in SQL stuff is usually indexed from 1. But I don't feel strongly about it.
Me neither - perfectly fine to align to 1 in both cases, just want them to be consistent is all. |
2 participants
Work on inlined primitive collections (VALUES)
moving it from RelationalTypeMappingPostprocessor to SqlPruner.
preserved ordering via _ord and specifies the type mapping.
of EF.Constant().
Fixes #36158
Fixes #36462
Fixes #36463
/cc @artl93