Squeryl/Record throws a RuntimeException when referencing the field of a pre-retrieved entity
32 views
Skip to first unread message
Dave Whittaker
Jan 13, 2011, 10:35:15 AM1/13/11
to Lift
This one had me digging in the squeryl and record source all day
yesterday. If you retrieve an entity and then try to use one of it's
fields in a subsequent query the code will compile but you'll get a
NoSuchElementException when you try to execute it. So something like:
val theOne = from(one)(o => where (o.idField === "one") select
(o)).head
val theMany = from(many)(m => where (m.oneId === theOne.idField)
select(m))
Will fail. The reason is that Squeryl instruments the parameters used
within a from statement and sets
FieldReferenceLinker._lastAccessedFieldReference when a Field var is
accessed on an instrumented object. Since theOne isn't passed into
the from it isn't instrumented,
FieldReferenceLinker._lastAccessedFieldReference is empty and code
like this in RecordTypeMode will result in the exception:
implicit def long2ScalarLong(l: MandatoryTypedField[Long]) =
new SelectElementReference[LongType]
(FieldReferenceLinker.takeLastAccessedFieldReference.get)
(createOutMapperLongType) with NumericalExpression[Long]
Notice the .get call on
FieldReferenceLinker.takeLastAccessedFieldReference which returns and
Option. Yes, using val theMany = from(many)(m => where (m.oneId ===
theOne.idField.is) select(m)) will work but it doesn't seem intuitive
to me at all that direct references to columns are ok in some
situations and not ok in others, especially when the implicit
conversion happens and the code doesn't fail until runtime. After
looking at the Squeryl code I saw that typically a
ConstantExpressionNode is created in situations where
FieldReferenceLinker._lastAccessedFieldReference is not available and
when I changed the above to:
implicit def long2ScalarLong(l: MandatoryTypedField[Long]):
NumericalExpression[LongType] =
FieldReferenceLinker.takeLastAccessedFieldReference match {
case Some(n: SelectElement) => new SelectElementReference[LongType]
(n)(createOutMapperLongType) with NumericalExpression[LongType]
case None => new ConstantExpressionNode[LongType](l.is) with
NumericalExpression[LongType]
}
It seemed to fix the problem. Is there a reason this isn't the way
that RecordTypeMode works now? I've changed all the
MandatoryTypedField implicits to this form and didn't notice any
issues in the testing that I did. If we can agree that this is a
change that should be made I'd be willing to update the
OptionalTypedFields as well and contribute the changes back.
-Dave
yesterday. If you retrieve an entity and then try to use one of it's
fields in a subsequent query the code will compile but you'll get a
NoSuchElementException when you try to execute it. So something like:
val theOne = from(one)(o => where (o.idField === "one") select
(o)).head
val theMany = from(many)(m => where (m.oneId === theOne.idField)
select(m))
Will fail. The reason is that Squeryl instruments the parameters used
within a from statement and sets
FieldReferenceLinker._lastAccessedFieldReference when a Field var is
accessed on an instrumented object. Since theOne isn't passed into
the from it isn't instrumented,
FieldReferenceLinker._lastAccessedFieldReference is empty and code
like this in RecordTypeMode will result in the exception:
implicit def long2ScalarLong(l: MandatoryTypedField[Long]) =
new SelectElementReference[LongType]
(FieldReferenceLinker.takeLastAccessedFieldReference.get)
(createOutMapperLongType) with NumericalExpression[Long]
Notice the .get call on
FieldReferenceLinker.takeLastAccessedFieldReference which returns and
Option. Yes, using val theMany = from(many)(m => where (m.oneId ===
theOne.idField.is) select(m)) will work but it doesn't seem intuitive
to me at all that direct references to columns are ok in some
situations and not ok in others, especially when the implicit
conversion happens and the code doesn't fail until runtime. After
looking at the Squeryl code I saw that typically a
ConstantExpressionNode is created in situations where
FieldReferenceLinker._lastAccessedFieldReference is not available and
when I changed the above to:
implicit def long2ScalarLong(l: MandatoryTypedField[Long]):
NumericalExpression[LongType] =
FieldReferenceLinker.takeLastAccessedFieldReference match {
case Some(n: SelectElement) => new SelectElementReference[LongType]
(n)(createOutMapperLongType) with NumericalExpression[LongType]
case None => new ConstantExpressionNode[LongType](l.is) with
NumericalExpression[LongType]
}
It seemed to fix the problem. Is there a reason this isn't the way
that RecordTypeMode works now? I've changed all the
MandatoryTypedField implicits to this form and didn't notice any
issues in the testing that I did. If we can agree that this is a
change that should be made I'd be willing to update the
OptionalTypedFields as well and contribute the changes back.
-Dave
Michael Gottschalk
Jan 13, 2011, 3:41:28 PM1/13/11
to Lift
Hi Dave!
On 13 Jan., 16:35, Dave Whittaker <d...@iradix.com> wrote:
> This one had me digging in the squeryl and record source all day
> yesterday.
Thank you for your efforts!
> If you retrieve an entity and then try to use one of it's
> fields in a subsequent query the code will compile but you'll get a
> NoSuchElementException when you try to execute it.
Yes, I see... that's surely not as expected.
Looks good to me.
I can't tell the reason why it was not implemented like that
initially. I assume nobody ran into that problem yet.
As far as I can tell, squeryl-record is not yet widely used, probably
because it did not really work until lift 2.2.
So I'm glad you are using it intensively now and report the bugs.
> I've changed all the
> MandatoryTypedField implicits to this form and didn't notice any
> issues in the testing that I did. If we can agree that this is a
> change that should be made I'd be willing to update the
> OptionalTypedFields as well and contribute the changes back.
Thank you for your offer, but becoming a lift contributor is a bit
involved (you have to sign an IP assignment).
I will fix it in the way you suggested and I'll also extend the unit
tests so that this case is covered.
I'd be glad, however, if you open another ticket for this issue.
It would also be cool if you update the squeryl-record wiki page with
the problems you encountered and the preliminary workarounds for them:
http://www.assembla.com/wiki/show/liftweb/Squeryl
Cheers,
Michael
On 13 Jan., 16:35, Dave Whittaker <d...@iradix.com> wrote:
> This one had me digging in the squeryl and record source all day
> yesterday.
> If you retrieve an entity and then try to use one of it's
> fields in a subsequent query the code will compile but you'll get a
> NoSuchElementException when you try to execute it.
I can't tell the reason why it was not implemented like that
initially. I assume nobody ran into that problem yet.
As far as I can tell, squeryl-record is not yet widely used, probably
because it did not really work until lift 2.2.
So I'm glad you are using it intensively now and report the bugs.
> I've changed all the
> MandatoryTypedField implicits to this form and didn't notice any
> issues in the testing that I did. If we can agree that this is a
> change that should be made I'd be willing to update the
> OptionalTypedFields as well and contribute the changes back.
involved (you have to sign an IP assignment).
I will fix it in the way you suggested and I'll also extend the unit
tests so that this case is covered.
I'd be glad, however, if you open another ticket for this issue.
It would also be cool if you update the squeryl-record wiki page with
the problems you encountered and the preliminary workarounds for them:
http://www.assembla.com/wiki/show/liftweb/Squeryl
Cheers,
Michael
Dave Whittaker
Jan 13, 2011, 4:53:32 PM1/13/11
to Lift
> Hi Dave!
>
Hi Michael. Thanks again for getting back to me
> Thank you for your efforts!
library.
> Looks good to me.
> I can't tell the reason why it was not implemented like that
> initially. I assume nobody ran into that problem yet.
> As far as I can tell, squeryl-record is not yet widely used, probably
> because it did not really work until lift 2.2.
> So I'm glad you are using it intensively now and report the bugs.
exercise both Squeryl and the Record implementation pretty well. I'll
definitely keep letting you know if I find any issues.
> Thank you for your offer, but becoming a lift contributor is a bit
> involved (you have to sign an IP assignment).
> I will fix it in the way you suggested and I'll also extend the unit
> tests so that this case is covered.
> I'd be glad, however, if you open another ticket for this issue.
http://www.assembla.com/spaces/liftweb/tickets/843-comparing-with-fields-of-objects-not-retrieved-in-current-squeryl-query-results-in-nosuchelemente---.
As a self employed developer it's probably easier for me than most to
sign an IP assignment and I wouldn't mind contributing when I can, but
we can always wait until I come across something that you don't have
the time to fix before heading down that road if you think it will be
difficult.
> It would also be cool if you update the squeryl-record wiki page with
> the problems you encountered and the preliminary workarounds for them:http://www.assembla.com/wiki/show/liftweb/Squeryl
-Dave
Dave Whittaker
Jan 13, 2011, 5:28:54 PM1/13/11
to Lift
Michael,
I've updated the wiki as requested. Let me know if there is anything
else I can do to help.
-Dave
On Jan 13, 4:53 pm, Dave Whittaker <d...@iradix.com> wrote:
> > Hi Dave!
>
> Hi Michael. Thanks again for getting back to me
>
> > Thank you for your efforts!
>
> You are quite welcome. Thanks for putting together such a great
> library.
>
> > Looks good to me.
> > I can't tell the reason why it was not implemented like that
> > initially. I assume nobody ran into that problem yet.
> > As far as I can tell, squeryl-record is not yet widely used, probably
> > because it did not really work until lift 2.2.
> > So I'm glad you are using it intensively now and report the bugs.
>
> I'm just getting started but the app I'm putting together should
> exercise both Squeryl and the Record implementation pretty well. I'll
> definitely keep letting you know if I find any issues.
>
> > Thank you for your offer, but becoming a lift contributor is a bit
> > involved (you have to sign an IP assignment).
> > I will fix it in the way you suggested and I'll also extend the unit
> > tests so that this case is covered.
> > I'd be glad, however, if you open another ticket for this issue.
>
> I appreciate you offering to make the update. I've createdhttp://www.assembla.com/spaces/liftweb/tickets/843-comparing-with-fie....
I've updated the wiki as requested. Let me know if there is anything
else I can do to help.
-Dave
On Jan 13, 4:53 pm, Dave Whittaker <d...@iradix.com> wrote:
> > Hi Dave!
>
> Hi Michael. Thanks again for getting back to me
>
> > Thank you for your efforts!
>
> You are quite welcome. Thanks for putting together such a great
> library.
>
> > Looks good to me.
> > I can't tell the reason why it was not implemented like that
> > initially. I assume nobody ran into that problem yet.
> > As far as I can tell, squeryl-record is not yet widely used, probably
> > because it did not really work until lift 2.2.
> > So I'm glad you are using it intensively now and report the bugs.
>
> I'm just getting started but the app I'm putting together should
> exercise both Squeryl and the Record implementation pretty well. I'll
> definitely keep letting you know if I find any issues.
>
> > Thank you for your offer, but becoming a lift contributor is a bit
> > involved (you have to sign an IP assignment).
> > I will fix it in the way you suggested and I'll also extend the unit
> > tests so that this case is covered.
> > I'd be glad, however, if you open another ticket for this issue.
>
Michael Gottschalk
Jan 13, 2011, 6:21:10 PM1/13/11
to Lift
Hi Dave,
very nice, thank you!
> Let me know if there is anything
> else I can do to help.
Just keep on testing the squeryl-record stuff.
I have already fixed both issues locally.
I'll try to get the fixes on review board tomorrow, so they should be
in master on Saturday or Sunday.
Cheers,
Michael
very nice, thank you!
> Let me know if there is anything
> else I can do to help.
I have already fixed both issues locally.
I'll try to get the fixes on review board tomorrow, so they should be
in master on Saturday or Sunday.
Cheers,
Michael
Reply all
Reply to author
Forward
0 new messages