Expressions compiler extension #1277
Merged
Conversation
This file contains 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
|
Maybe we should add: ifFastFailedReturnNull = true to the compile fast. So we also get the expressions wich fail (otherwise fallback to compile is used (if I read correctly)) |
|
No, this flag controls behavior when unsupported expression node found in expression. In our case it just generate incorrect code. |
|
What I don't like on this patches, is that we introduce more static settings... |
|
It is just a test branch, which will be discarded. We can think how to do it if we decide to support compiler customization. |
|
only wanted to say that! I think we still should think about how we could remove most of the static settings... |
|
@MaceWindu I've fixed all linq2db exception in FastExpressionCompiler. So you can check again. |
|
@jogibear9988, Right now I'm busy preparing 2.3.0 release so maybe you can do it? Just run tests for databases you have. |
|
@jogibear9988 what is wrong with static settings? |
|
For example you use linq2db in different projects with different settings. And these two projects are then used in another project. Or you need different settings in some places at the code, and some need other. |
MaceWindu
commented
Aug 31, 2018
| } | ||
|
|
||
| internal static TDelegate CompileExpression<TDelegate>(this Expression<TDelegate> expression) | ||
| where TDelegate : Delegate |
There was a problem hiding this comment.
this constraint needs C# 7.3, now it fails travis build
There was a problem hiding this comment.
you introduced this. :-) see 201e094
There was a problem hiding this comment.
Yep, just making notes. Looks like Travis environment will need some updates
|
@MaceWindu if done a few more changes. But atm, I could not run the linq2db tests, need to look why. But why don‘t the tests run on Appveyor? |
|
Maybe you could test again with newest FastExpression Parser? |
|
|
And year, for sqlite one test fails before crash: |
|
About running tests - I think you can add FEC source file as link to test project to avoid Appveyor errors, because now to test it I buid it and sign using linq2db key. |
|
Try again with newest FEC. Looks like many more tests work now. |
|
Edit: sory, tested wrong version. Will retest ok, retested with latest version: And SQLite testrun still hangs on EnumMapInsertFromSelectWithParam3 |
|
Now it Looks better |
|
But the EnumMapInsertFromSelectWithParam3 still fails. Exception is "Access Violation Exception", seams invalid IL is generated. But for this to find out, I have to find where the Expression for the IL is generated! The IL looks like this: (it's very big ;-) |
|
@MaceWindu don't know why the test hangs forever, crashes with exception! |
|
it doesn't hang really. Visual studio test runner waits for test to finish, but because test agent process crashed without a chance to report error to host, VS is not aware of it |
|
This is the Debug view of the failing Expression Tree: |
well, just put breakpoint on compiler call and inspect passed expression (could be multiple calls within one test with different expressions) |
|
BTW, don't fully trust debug view. It could have rendering bugs. At least it doesn't handle \0 characters properly in string constants. Also it doesn't consistent with variables naming - it could name different variables with the same name sometimes or name them as parameters |
|
I found the Expression, but it's huge. I now have to create a test, then try to track down whats wrong. Will take some time! |
|
ok. |
|
@MaceWindu Maybe once more again when my pull req is merged. With my newest patched Version it seems to work |
|
/azp run test-all |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run test-all |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run test-all |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run test-all |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run test-all |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Test baselines changed by this PR. Don't forget to merge/close baselines PR after this pr merged/closed. |
|
/azp run test-all |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run test-all |
|
Azure Pipelines successfully started running 1 pipeline(s). |
MaceWindu
pushed a commit
to linq2db/linq2db.baselines
that referenced
this pull request
Feb 4, 2021
* [Windows / NET 5.0 / SQL Server 2019 (Microsoft.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2019 (System.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2005 (System.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2019 (Microsoft.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2012 (System.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2008 (System.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2014 (System.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2017 (System.Data.SqlClient)] baselines * [Windows / NET472 / SQL Server 2016 (System.Data.SqlClient)] baselines * [Windows / NETCOREAPP2.1 / SQL Server 2005 (System.Data.SqlClient)] baselines * [Windows / NETCOREAPP2.1 / SQL Server 2012 (System.Data.SqlClient)] baselines * [Windows / NETCOREAPP2.1 / SQL Server 2008 (System.Data.SqlClient)] baselines * [Windows / NETCOREAPP2.1 / SQL Server 2014 (System.Data.SqlClient)] baselines * [Windows / NETCOREAPP2.1 / SQL Server 2017 (System.Data.SqlClient)] baselines * [Windows / NETCOREAPP2.1 / SQL Server 2016 (System.Data.SqlClient)] baselines Co-authored-by: Azure Pipelines Bot <[email protected]>
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.
Experemental stream to see if it makes sense to add extension point to replace expression compiler with something else. E.g. https://github.com/dadhi/FastExpressionCompiler
Doesn't make much sense right now as FastExpressionCompiler pretty unusable right now:
I plan to report those issues and we can get back to this PR when they are fixed