Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactoring LINQ Query parsing. #3401

Merged
merged 1,042 commits into from
May 29, 2024
Merged

Refactoring LINQ Query parsing. #3401

merged 1,042 commits into from
May 29, 2024

Conversation

sdanyliv
Copy link
Member

@sdanyliv sdanyliv commented Jan 13, 2022 •
edited by MaceWindu
Loading

Well, trying to simplify things and avoid any workarounds.

List of fixed issues (incomplete)

Tasks todo:

  • Ensure that CTE do not hold DataContext instance in cache.
  • MySql provider needs version support. Window Functions are introduced from version 8.0
  • Sybase provider needs version support. Window Functions are not supported in ASE
  • Firebird provider needs version support. Window Functions are introduced from version 3.0

New features:

  • There is new way how to define GetTable call for TableFunctionAtrribute - TableFromExpression
[Sql.TableFunction(Name="GetParentByID")]
public ITable<Parent> GetParentByID(int? id)
{
    return _ctx.TableFromExpression(() => GetParentByID(id));
}

Instead of:

[Sql.TableFunction(Name="GetParentByID")]
public ITable<Parent> GetParentByID(int? id)
{
    var methodInfo = typeof(FunctionsOld).GetMethod("GetParentByID", new [] {typeof(int?)})!;

    return _ctx.GetTable<Parent>(this, methodInfo, id);
}

Breaking changes:

  • SQL AST refactored.

@sdanyliv sdanyliv marked this pull request as draft January 13, 2022 14:33
@jods4 jods4 mentioned this pull request Feb 6, 2022
Closed
@MaceWindu MaceWindu added this to the 4.0.0 milestone Mar 3, 2022
@MaceWindu MaceWindu modified the milestones: 4.1.0, Future Mar 27, 2022
@MaceWindu MaceWindu changed the base branch from dev.v4 to master April 7, 2022 12:43
@MaceWindu MaceWindu modified the milestones: Future, 5.0.0 May 13, 2022
jods4
jods4 reviewed Sep 21, 2022
View reviewed changes
Source/LinqToDB/Expressions/ExpressionVisitors/TransformVisitor.cs Outdated
@@ -188,7 +188,64 @@ public static TransformVisitor<TContext> Create(TContext context, Func<TContext,

// ReSharper disable once InconsistentNaming
[MethodImpl(MethodImplOptions.AggressiveInlining)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the size of the new implementation of this method, I think you want to remove the AggressiveInlining hint.

viceroypenguin
viceroypenguin reviewed Oct 4, 2022
View reviewed changes
Source/LinqToDB/DataProvider/SqlServer/SqlServerSqlBuilder.cs
var alias = GetTableAlias(table);
if (alias == null)
{
throw new InvalidOperationException();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have an exception message here.

@MaceWindu MaceWindu mentioned this pull request Jul 29, 2023
Merged
@linq2db linq2db deleted a comment from azure-pipelines bot Oct 13, 2023
@linq2db linq2db deleted a comment from azure-pipelines bot Oct 16, 2023
MaceWindu
MaceWindu reviewed Oct 28, 2023
View reviewed changes
Source/LinqToDB/Linq/Builder/Visitors/ExposeExpressionVisitor.cs Outdated
case ExpressionType.ArrayLength:
{
//TODO: WTF?
throw new NotImplementedException();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WTF? :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope some test will pass this line. Extracted and removed code from some previous expression transformation code.

MaceWindu
MaceWindu reviewed Oct 28, 2023
View reviewed changes
Source/LinqToDB/Linq/Builder/ExpressionBuilder.Generation.cs Outdated
var currentRef = SequenceHelper.EnsureType(rootReference, onType);
var member = currentRef.Type.GetMemberEx(discriminatorMemberInfo);
member = discriminatorMemberInfo;
if (false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: unreachable code

@MaceWindu MaceWindu mentioned this pull request Oct 28, 2023
Merged
MaceWindu
MaceWindu reviewed Oct 30, 2023
View reviewed changes
Source/LinqToDB/Common/Internal/TaskCache.cs Outdated
@@ -12,6 +12,11 @@ internal static class TaskCache
public static readonly Task<int> Zero = Task.FromResult(0);
public static readonly Task<int> MinusOne = Task.FromResult(-1);

#if NET45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be gone before release as we will bump lowest supported NETFX version from 4.5 to 4.6.2

@viceroypenguin
Copy link
Contributor

/azp run test-all

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

viceroypenguin
viceroypenguin reviewed May 20, 2024
View reviewed changes
Source/LinqToDB/Linq/Builder/ExpressionBuilder.SqlBuilder.cs

default :
var predicate = ConvertPredicate(context, expression);
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three commented sections to leave/remove/etc.

@viceroypenguin
Copy link
Contributor

/azp run test-all

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sdanyliv
Copy link
Member Author

/azp run test-all

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@viceroypenguin
Copy link
Contributor

/azp run test-all

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

viceroypenguin
viceroypenguin reviewed May 27, 2024
View reviewed changes
Source/LinqToDB/Linq/Builder/LoadWithBuilder.cs
};
var prevSequence = (LoadWithContext)sequence;
if (prevSequence.LastLoadWithInfo == null)
throw new InvalidOperationException();
Copy link
Contributor

@viceroypenguin viceroypenguin May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add error message? (x2 more on L142 and L161)

viceroypenguin
viceroypenguin reviewed May 27, 2024
View reviewed changes
Source/LinqToDB/Linq/Builder/MergeBuilder.MergeContext.cs
{
var root = Builder.GetRootObject(expression);
if (OutputExpression == null || OutputContext == null)
throw new InvalidOperationException();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add error message?

viceroypenguin
viceroypenguin reviewed May 27, 2024
View reviewed changes
Source/LinqToDB/Linq/Builder/MergeBuilder.cs
Sequence[2].SelectQuery = Sequence[0].SelectQuery;
}
if (targetKeySelector == null || sourceKeySelector == null)
throw new InvalidOperationException();
Copy link
Contributor

@viceroypenguin viceroypenguin May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add error message? (and L162, L192)

viceroypenguin
viceroypenguin reviewed May 27, 2024
View reviewed changes
Source/LinqToDB/Linq/Builder/SequenceHelper.cs
return methodCall.Arguments[idx].UnwrapLambda();
}

//TODO: I don't like this. Hints are like mess. Quick workaround before review
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time to address the TODO?

viceroypenguin
viceroypenguin reviewed May 27, 2024
View reviewed changes
Source/LinqToDB/Linq/Builder/TableLikeQueryContext.cs
var targetCloned = cloningContext.CloneContext(TargetContextRef.BuildContext);

if (ConnectionLambda == null)
throw new InvalidOperationException();
Copy link
Contributor

@viceroypenguin viceroypenguin May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add error message? (and L333)

@viceroypenguin
Copy link
Contributor

/azp run test-all

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

MaceWindu pushed a commit to linq2db/linq2db.baselines that referenced this pull request May 29, 2024
@linq2dbot
Baselines for linq2db/linq2db#3401 (#1215)
6499e8f 
* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / SQL Server EXTRAS] baselines

* [Linux / Oracle 23c] baselines

* [Linux / SQL Server 2019] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / SQL Server EXTRAS] baselines

* [Linux / SQL Server 2019] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / SQL Server EXTRAS] baselines

* [Linux / SQL Server 2019] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / SQL Server EXTRAS] baselines

* [Linux / SQL Server 2019] baselines

* [Windows / Access MDB (Jet/ODBC)] baselines

* [Windows / Access ACE (OLEDB/ODBC) x86] baselines

* [Windows / SQL Server EXTRAS] baselines

* [Linux / SQL Server 2019] baselines

---------

Co-authored-by: Azure Pipelines Bot <[email protected]>
@MaceWindu MaceWindu merged commit 7ec71d5 into version_6 May 29, 2024
67 checks passed
@MaceWindu MaceWindu deleted the refactor/parsing branch May 29, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jods4 jods4 jods4 left review comments

@viceroypenguin viceroypenguin viceroypenguin approved these changes

@MaceWindu MaceWindu MaceWindu left review comments

@igor-tkachev igor-tkachev Awaiting requested review from igor-tkachev

@Shane32 Shane32 Awaiting requested review from Shane32

Assignees
No one assigned
Labels
None yet
Milestone
6.0.0-preview.1
6 participants
@sdanyliv @MaceWindu @linq2dbot @viceroypenguin @jods4 @jogibear9988