Endurable.Range isn't getting parsed by re-linq properly?

8 views
Skip to first unread message

Gordon Watts

unread,
Feb 27, 2016, 8:24:39 AM2/27/16
to re-moti...@googlegroups.com

Hi,

  In the process of hunting down a bug of mine that (might) be in re-linq, I upgraded to re-linq 2.0. I’ve got what looks like a bug to me.

 

  Consider the following LINQ query:

 

            var q = new QueriableDummy<dummyntup>();

            var r1 = from evt in q

                     from mr in Enumerable.Range(0, evt.run)

                     where mr > 3

                     select mr;

            var r = r1.Count();

 

  If I use the QueryModel dump to console I get:

 

        from dummyntup evt in value(LINQToTTreeLib.Tests.QueriableDummy`1[LINQToTTreeLib.Tests.dummyntup]) from Int32 mr in [110003] where ([mr] > 3) select [mr] => Count()

 

  This looks fine. I have, however, created a MethodCall IExpressionTransformer that looks for Enumerable.Range and translates it. The code looks like the following:

 

          public Expression Transform(MethodCallExpression expression)

        {

            if (expression.Object == null && expression.Method.Name == "Range" && expression.Arguments.Count == 2)

            {

                if (expression.Method.DeclaringType == typeof(System.Linq.Enumerable))

                {

                    return new EnumerableRangeExpression(expression.Arguments[0], expression.Arguments[1]);

                }

            }

            return expression;

        }

 

  The problem is that Arguments[1] has in it “{evt.run}” (as expected), but “evt” is a parameter, not a query reference expression – which is should be, I believe.

 

  The call stack is deep in the middle of re-linq’s parsing (see attached file if you wish).

 

  I suspect this is a bug in re-linq. This behavior was not present in the 1.x versions of re-linq. This is me following up on one of my failing tests after upgrading. At the very least, it is a behavior change, in which case I need advice on how to hook “evt” up with the proper query reference expression.

 

Cheers,
  Gordon

 

P.S. The upgrade was pretty smooth. Other than a number of name changes, everything seems to be behaving as expected.

 

P.P.S. The other bug I’m chasing, if you want to know, is that QueryModel.Clone seems to be throwing an exception that I don’t understand. But I’ve not gotten far enough to isolate that problem yet.

Enumerable.Range.Bad.QRE.Debug.txt

Michael Ketting

unread,
Feb 28, 2016, 4:19:13 PM2/28/16
to re-motion Users
Hi Gordon!

I've got it: When you did the upgrade, you missed the condition in EnumerableRangeExpression.VisitChildren. You may only return "this" when none of the children have changed, but you did an "OR" instead of an "AND" when comparing the old and new values of the HighBoundary and LowBoundary. ReferenceQuerySourceExpression got inserted just fine after I fixed this.

Glad to hear you had a mostly smooth upgrade experience :)

Best regards, Michael

Gordon Watts

unread,
Feb 29, 2016, 2:27:09 AM2/29/16
to Michael Ketting, re-motion Users

Oi! That is embarassing! I wouldn’t have thought to look there, but it makes sense in retrospect.

 

BTW, in this case, if only the high boundary changes, do I need to clone the low broundry before creating the new eumerable range expression? Or is it ok to just re-used the unchanged expression (as I am now).

 

Cheers,
Gordon.

--
You received this message because you are subscribed to the Google Groups "re-motion Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to re-motion-use...@googlegroups.com.
To post to this group, send email to re-moti...@googlegroups.com.
Visit this group at https://groups.google.com/group/re-motion-users.
For more options, visit https://groups.google.com/d/optout.

Michael Ketting

unread,
Feb 29, 2016, 2:44:06 AM2/29/16
to re-motion Users, michael...@rubicon.eu
:) No worries, I saw it but missed the implication on the first read, too. It was more of a hunch once I saw when the ParameterExpression was supposed to get replaced with a theQuerySourceReferenceExpression.

Normally, no you should be okay the way you're doing it. Expressions are designed to be immutable so you can just reuse them as needed but you do need to visit the nested expressions as you are doing right now. There is one caveat - the  SubQueryExpressions contains a QueryModel which is unfortunately mutable (the things you learn to avoid as you get older and wiser...). Most of the time, you're still fine, though. It only becomes an issue when you actually need to fork the original QueryModel to do two different things with it.

Best regards, Michael
To unsubscribe from this group and stop receiving emails from it, send an email to re-motion-users+unsubscribe@googlegroups.com.
To post to this group, send email to re-motion-users@googlegroups.com.

Gordon Watts

unread,
Feb 29, 2016, 12:36:48 PM2/29/16
to Michael Ketting, re-motion Users, michael...@rubicon.eu

Hi,

  Ha! And of course, the Concat stuff is forking it! 😊

To unsubscribe from this group and stop receiving emails from it, send an email to re-motion-use...@googlegroups.com.
To post to this group, send email to re-moti...@googlegroups.com.

Michael Ketting

unread,
Feb 29, 2016, 3:54:30 PM2/29/16
to re-motion Users
Yep, that's what I'm a tad worried about in regards to the bug-report you did.

Best regards, Michael


On Monday, February 29, 2016 at 6:36:48 PM UTC+1, Gordon Watts wrote:

Hi,

  Ha! And of course, the Concat stuff is forking it! 😊

Reply all
Reply to author
Forward
0 new messages