lift / framework

Lift Framework
http://liftweb.net
Apache License 2.0
1.27k stars 278 forks source link

Record-Squeryl selecting unspecified columns in generated SQL #876

Closed lunaspeed closed 13 years ago

lunaspeed commented 13 years ago

Record-Squeryl when selecting specific column, statement includes other unspecified columns. Even though result is correct, when used in a "IN" statement will cause SQL exception

caused by: implicit conversions defined in RecordTypeMode are not applied for arguments of the select function

work around: from(KKSchema.textbooks) (b => select(b.idField.is)) or from(KKSchema.textbooks) (b => select(b.bookName.is))

Reported in this thread: http://groups.google.com/group/liftweb/browse_thread/thread/be5864b4768c12a4

max-l commented 13 years ago

First, another work around : from(books)(b => where(b.name === "Pillars Of The Earth") select(b.idField.value))

Here's what's happening :

user code can put whatever they want in the select clause, and Squeryl must determine which columns need to go in the SQL's select clause, ex, if the query is : from(books)(b => where(b.name === "Pillars Of The Earth") select(b).single

we need all columns from book, if it is :

from(books)(b => where(b.name === "Pillars Of The Earth") select((b.id, b.name)).single

we only need two, etc...

Squeryl does this by :

1) analysing the object graph that is returned by the select closure, this is done by this method :

FieldReferenceLinker.determineColumnsUtilizedInYeldInvocation(q: QueryExpressionNode[_], rsm: ResultSetMapper, selectClosure: ()=>AnyRef)

2) Intercepting property accessors of the "sample row object", to explain what I mean by "sample object", in the following query :

from(books)(b => where(b.name === "Pillars Of The Earth") select(b.id).single

the variable 'b' is points to the sample object. It is constructed at query definition time, and it happend to be a subclass of the table's class (Book in this example). The subclass is created dynamically by CGLIB, who allows all of it's methods to be interceptable. Calls to the sample object's properties are intercepted by :

FieldReferenceLinker.PosoPropertyAccessInterceptor

and the accessed fields go in YieldInspection._utilizedFields... To make a long story short, this algo works pretty well PrimitiveMode and CustomTypeMode, but it turns out that Record Fields are a different beast, in that they hold a reference back to FieldReferenceLinker.determineColumnsUtilizedInYeldInvocation, who when it analyses the returned field, detects that the sample object is utilised in the yield closure, so it needs to assign all of it's columnds...

An easy fix for this would be to put a hack in FieldReferenceLinker.determineColumnsUtilizedInYeldInvocation to prevent it from following this pointer :

def owner: OwnerType

in trait OwnedField, for example :


class DateTimeField[OwnerType <: Record[OwnerType]](rec: OwnerType)
  extends Field[Calendar, OwnerType] with MandatoryTypedField[Calendar] with DateTimeTypedField {

  def owner = rec
...

'rec' points to the onwed 'sample' object, and the later must be excluded from the yield closure returned graph (what I call yield inspection in the code...).

Anyhow... I am looking into this, the "yield inspection" will probably need to be delegated in part to a squeryl-record class, so that it can deal with the fact that a net.liftweb.record.OwnedField has a back pointer to it's Record object...

... While explaining all this to you (...and to myself) I hacked this solution below, the extra condition (in red) does the trick... but you will agree with me that it is hackish...to say the least...

private def _populateSelectColsRecurse(visited: HashSet[Int] , yi: YieldInspection,q: QueryExpressionElements, o: AnyRef):Unit = {

val idHashCode = System.identityHashCode(o)

if(o == null || o.getClass.getName.startsWith("java.")  || visited.contains(idHashCode))
  return

//visited.add(o)
visited.add(idHashCode)

_populateSelectCols(yi, q, o)
for(f <-o.getClass.getDeclaredFields) {
  f.setAccessible(true);
  val ob = f.get(o)

  // don't follow closures
  if(! f.getType.getName.startsWith("scala.Function")  && ! f.getName.equals("rec")) {
    _populateSelectColsRecurse(visited, yi, q, ob)
  }
}

}

It would be cleaner (and more correct) to have :

if(! f.getType.getName.startsWith("scala.Function") && ! FieldMetaDataFactory.hideFromYieldInspection(o, f)) {

and then, within the squeryl-record adapter :

class RecordMetaDataFactory { ... def hideFromYieldInspection(o: AnyRef, f: Field) = o.isInstanceOf[OwnedField] && f.getName.equals("ref") }

max-l commented 13 years ago

First, another work around : from(books)(b => where(b.name === "Pillars Of The Earth") select(b.idField.value))

Here's what's happening :

user code can put whatever they want in the select clause, and Squeryl must determine which columns need to go in the SQL's select clause, ex, if the query is : from(books)(b => where(b.name === "Pillars Of The Earth") select(b).single

we need all columns from book, if it is :

from(books)(b => where(b.name === "Pillars Of The Earth") select((b.id, b.name)).single

we only need two, etc...

Squeryl does this by :

1) analysing the object graph that is returned by the select closure, this is done by this method :

FieldReferenceLinker.determineColumnsUtilizedInYeldInvocation(q: QueryExpressionNode[_], rsm: ResultSetMapper, selectClosure: ()=>AnyRef)

2) Intercepting property accessors of the "sample row object", to explain what I mean by "sample object", in the following query :

from(books)(b => where(b.name === "Pillars Of The Earth") select(b.id).single

the variable 'b' is points to the sample object. It is constructed at query definition time, and it happend to be a subclass of the table's class (Book in this example). The subclass is created dynamically by CGLIB, who allows all of it's methods to be interceptable. Calls to the sample object's properties are intercepted by :

FieldReferenceLinker.PosoPropertyAccessInterceptor

and the accessed fields go in YieldInspection._utilizedFields... To make a long story short, this algo works pretty well PrimitiveMode and CustomTypeMode, but it turns out that Record Fields are a different beast, in that they hold a reference back to FieldReferenceLinker.determineColumnsUtilizedInYeldInvocation, who when it analyses the returned field, detects that the sample object is utilised in the yield closure, so it needs to assign all of it's columnds...

An easy fix for this would be to put a hack in FieldReferenceLinker.determineColumnsUtilizedInYeldInvocation to prevent it from following this pointer :

def owner: OwnerType

in trait OwnedField, for example :


class DateTimeField[OwnerType <: Record[OwnerType]](rec: OwnerType)
  extends Field[Calendar, OwnerType] with MandatoryTypedField[Calendar] with DateTimeTypedField {

  def owner = rec
...

'rec' points to the onwed 'sample' object, and the later must be excluded from the yield closure returned graph (what I call yield inspection in the code...).

Anyhow... I am looking into this, the "yield inspection" will probably need to be delegated in part to a squeryl-record class, so that it can deal with the fact that a net.liftweb.record.OwnedField has a back pointer to it's Record object...

... While explaining all this to you (...and to myself) I hacked this solution below, the extra condition (in red) does the trick... but you will agree with me that it is hackish...to say the least...

private def _populateSelectColsRecurse(visited: HashSet[Int] , yi: YieldInspection,q: QueryExpressionElements, o: AnyRef):Unit = {

val idHashCode = System.identityHashCode(o)

if(o == null || o.getClass.getName.startsWith("java.")  || visited.contains(idHashCode))
  return

//visited.add(o)
visited.add(idHashCode)

_populateSelectCols(yi, q, o)
for(f <-o.getClass.getDeclaredFields) {
  f.setAccessible(true);
  val ob = f.get(o)

  // don't follow closures
  if(! f.getType.getName.startsWith("scala.Function")  && ! f.getName.equals("rec")) {
    _populateSelectColsRecurse(visited, yi, q, ob)
  }
}

}

It would be cleaner (and more correct) to have :

if(! f.getType.getName.startsWith("scala.Function") && ! FieldMetaDataFactory.hideFromYieldInspection(o, f)) {

and then, within the squeryl-record adapter :

class RecordMetaDataFactory { ... def hideFromYieldInspection(o: AnyRef, f: Field) = o.isInstanceOf[OwnedField] && f.getName.equals("ref") }

max-l commented 13 years ago

....accidentally hit submit ....twice while trying to clean the formating..... I hate ticketing systems that don't allow deleting comments......

migo commented 13 years ago

Fixed in 2.3-RC1.

github-importer commented 12 years ago

Imported from Assembla: http://www.assembla.com/spaces/liftweb/tickets/876