Re: [re-motion-users] I find a bug in ExpressionTreeParser.cs hope fix it

[Fabian Schmied]

Hi,

Thanks for your report. I've never used it, but it seems that Dynamic
Linq creates lambda expressions with parameters that have empty names,
which is very strange. Could you please check that this is really the
case? Just dump out the expression tree generated by DynamicLinq using
ToString() and post it here, please.

If this is indeed the reason for the exception, I'd ask you to create
a query parser integration test exhibiting the problem. You can use
Remotion.Linq.UnitTests.Linq.Core.Parsing.Structure.QueryParserIntegrationTests.VBSpecificQueryParserIntegrationTest
as a template.

As an additional question, why are you using the LinqToSqlAdapter
(instead of Microsoft's LinqToSql)? The LinqToSqlAdapter is meant only
as an example implementation of a LINQ provider based on re-linq's SQL
backend. I'm just saying this so that you know it's probably not
feature complete.

Thanks, best regards,
Fabian

On Thu, Oct 11, 2012 at 5:06 AM, 李永京 <leeyo...@gmail.com> wrote:
> I used Dynamic
> Linq(http://weblogs.asp.net/scottgu/archive/2008/01/07/dynamic-linq-part-1-using-the-linq-dynamic-query-library.aspx
> ) in LinqToSqlAdapter,
> example:
> var result = Db.Customers
> .Where("UnitPrice> @0", 10m)
> .GroupBy("new (it.CategoryID)", "it")
> .Select("new (Key as GroupKey, Max(UnitPrice) as
> MaxPrice)");
>
> and throw a Exception: associatedIdentifier is null;
>
> in ExpressionTreeParser.cs find Method:
>
> private IExpressionNode ParseNode (Expression expression, string
> associatedIdentifier)
> {
> if (associatedIdentifier == null)
> associatedIdentifier = _identifierGenerator.GetUniqueIdentifier
> ("<generated>_");
>
> var methodCallExpression = GetQueryOperatorExpression(expression);
> if (methodCallExpression != null)
> return ParseMethodCallExpression (methodCallExpression,
> associatedIdentifier);
> else
> return ParseNonQueryOperatorExpression(expression,
> associatedIdentifier);
> }
>
> instead of" if (string.IsNullOrEmpty(associatedIdentifier))" fix it
>
> --
> You received this message because you are subscribed to the Google Groups
> "re-motion Users" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/re-motion-users/-/EtUy2Afxm1EJ.
> To post to this group, send email to re-moti...@googlegroups.com.
> To unsubscribe from this group, send email to
> re-motion-use...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/re-motion-users?hl=en.

[李永京]

Hi,

I'm submit a patch for this test.

additional ,

I study re-linq implementation prepare write a linq to mysql implementation using re-linq

在 2012年10月11日星期四UTC+8下午2时52分15秒，Fabian Schmied写道：

[Fabian Schmied]

Hi,

> I'm submit a patch for this test.

Thanks for the patch, I've created a JIRA issue:
"https://www.re-motion.org/jira/browse/RM-5107".
The problem is that DynamicLinq creates LambdaExpressions with empty
parameter names, which is not possible in C#, which is why we haven't
supported it.

It should be easy to ímplement this, e.g., using your fix. However, I
first need a query-parser level test for this. If you've got the time,
I'd appreciate if you could write a test similar to
Remotion.Linq.UnitTests.Linq.Core.Parsing.Structure.QueryParserIntegrationTests.VBSpecificQueryParserIntegrationTest
for the specific issue (LambdaExpressions wth empty parameter names).
That would allow me to fix the issue faster.

Best regards,
Fabian

> https://groups.google.com/d/msg/re-motion-users/-/RTSYv1hhHfMJ.

[Alexander I. Zaytsev]

Hi, all

I believe that this should also fix this bug from NHibernate: https://nhibernate.jira.com/browse/NH-3239

2012/10/11 Fabian Schmied <fabian....@gmail.com>

[Fabian Schmied]

> I believe that this should also fix this bug from NHibernate:
> https://nhibernate.jira.com/browse/NH-3239

I don't know for sure, but I think NHibernate calculates its cache key
_before_ re-linq gets involved, doesn't it? In that case, our fixing
this in re-linq wouldn't solve the problem within NHibernate.

Best regards,
Fabian

[Alexander I. Zaytsev]

Hi,

2012/10/12 Fabian Schmied <fabian....@gmail.com>

> I believe that this should also fix this bug from NHibernate:
> https://nhibernate.jira.com/browse/NH-3239

I don't know for sure, but I think NHibernate calculates its cache key
_before_ re-linq gets involved, doesn't it?

Yes and no... The NHibernate until yesterday did the double work by setting names to parameters with empty names, because re-linq requires to parameter to be named.

And this was been making _before_ key calculation. I've moved the naming unnamed parameters after the key calculation. 

 

In that case, our fixing this in re-linq wouldn't solve the problem within NHibernate.

 But I feel that this part will become unnecessary when re-linq this bug will be fixed in re-linq.

[Fabian Schmied]

[...]

> Yes and no... The NHibernate until yesterday did the double work by setting
> names to parameters with empty names, because re-linq requires to parameter
> to be named.
>
> And this was been making _before_ key calculation. I've moved the naming
> unnamed parameters after the key calculation.

[...]

> But I feel that this part will become unnecessary when re-linq this bug
> will be fixed in re-linq.

Ah, I see. You're right, of course.

Cheers,

[Fabian Schmied]

FYI, I've fixed this issue in re-linq 1.13.171 (due this week) by
automatically generating identifier names for parameters without
names.

Best regards,
Fabian

[Alexander I. Zaytsev]

Thanks, where can I get official binaries? It is not available on http://relinq.codeplex.com yet

2012/10/17 Fabian Schmied <fabian....@gmail.com>

[Fabian Schmied]

Hi,

> Thanks, where can I get official binaries? It is not available on
> http://relinq.codeplex.com yet

Yeah, by "1.13.171 (due this week)" I meant that binaries would become
available this week on CodePlex and NuGet.
We usually publish our builds on Friday, but this week we might be
earlier. You can sign up for release notifications on CodePlex, so
that you'll be informed when we publish the build.

Best regards,
Fabian

[yjinglee]

Thanks Fabian.

I see you use official dynamic.cs file for this bug test..

the official dynamic.cs file parse new that generate memberInit expression, I modify it instead of generate new expression using dynamicClass constructor

Expression.New(type.GetConstructor(properties.Select(p => p.Type).ToArray()), expressions);

and edit ClassFactory class add generate constructor method

        static void GenerateConstructor(TypeBuilder tb, IList<FieldInfo> fieldInfos)

        {

            var builder = tb.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, fieldInfos.Select(p => p.FieldType).ToArray());

            var genGet = builder.GetILGenerator();

            genGet.Emit(OpCodes.Ldarg_0);

            genGet.Emit(OpCodes.Call, tb.BaseType.GetConstructor(System.Type.EmptyTypes));

            genGet.Emit(OpCodes.Nop);

            genGet.Emit(OpCodes.Nop);

            for (var i = 0; i < fieldInfos.Count; i++)

            {

                var op = OpCodes.Ldarg_S;

                switch (i)

                {

                    case 0:

                        op = OpCodes.Ldarg_1;

                        break;

                    case 1:

                        op = OpCodes.Ldarg_2;

                        break;

                    case 2:

                        op = OpCodes.Ldarg_3;

                        break;

                }

                genGet.Emit(OpCodes.Ldarg_0);

                genGet.Emit(op);

                genGet.Emit(OpCodes.Stfld, fieldInfos[i]);   

            }

            genGet.Emit(OpCodes.Nop);

            genGet.Emit(OpCodes.Ret);

        }

在 2012年10月17日星期三UTC+8下午9时44分55秒，Fabian Schmied写道：

[Fabian Schmied]

Hi,

> I see you use official dynamic.cs file for this bug test..
>
> the official dynamic.cs file parse new that generate memberInit expression,
> I modify it instead of generate new expression using dynamicClass
> constructor

[...]

Yes, I've noticed that. However, we're planning to implement support
for MemberInitExpressions in the SQL backend anyway, at some time in
the future ("https://www.re-motion.org/jira/browse/RM-2971").

Best regards,
Fabian

> https://groups.google.com/d/msg/re-motion-users/-/NxZzqFkWTgsJ.

[Guillaume Lecomte]

I implemented MemberInitExpression in Sqlbackend but it's not perfect yet, 

I was wondering if there is an easy way to contribute (see my post on: https://groups.google.com/forum/?hl=en#!forum/re-motion-dev)

Thanks in advance, 

Guillaume

[Michael Ketting]

Hello Guillaume!

Thanks for the post. Rest of the discussion is over on the dev-list :)

Best regards, Michael