Incomprehensible Error Message Passing Partial Evaluated Lambda Function

Details

Type: Bug
Status: Open
Priority: Major
Resolution: Unresolved
Affects Version/s: None
Fix Version/s: None
Component/s: None
Labels:
None

Number of attachments :
0

Description

When executing the code:

#! /usr/bin/env groovy

import groovy.sql.DataSet
import groovy.sql.Sql

@Grab ( 'org.xerial:sqlite-jdbc:3.7.2' )
@GrabConfig ( systemClassLoader = true )
def database
final words = [ ]
try {
  database = Sql.newInstance ( 'jdbc:sqlite:database.db' , 'org.sqlite.JDBC' )
  final wordsTable = new DataSet ( database , 'words' )
  ( 0 ..< 4 ).each { i ->
    words << wordsTable.findAll ( { j , item -> item.id == j }.curry ( i ) ).firstRow ( ).word
  }
}
finally {
  database?.close ( )
}
println words.join ( '' )

the result is the error:

Caught: groovy.lang.GroovyRuntimeException: Could not find the ClassNode for MetaClass: groovy.lang.MetaClassImpl@6c1826dc[class org.codehaus.groovy.runtime.CurriedClosure]
groovy.lang.GroovyRuntimeException: Could not find the ClassNode for MetaClass: groovy.lang.MetaClassImpl@6c1826dc[class org.codehaus.groovy.runtime.CurriedClosure]

which doesn't really tell the programmer anything useful about the executed code they wrote.

Issue Links

relates to

GROOVY-5377 CLONE - Incomprehensible Error Message Passing Partial Evaluated Lambda Function (improved error message for normal closure not on classpath case)

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

blackdrag blackdrag added a comment - 20/Mar/12 7:57 PM - edited

The exception means it could not find a source file which is needed, because findAll works on the source level and for that the source needs to be re-evaluated. Now here it is Closure on which curry was used. I think DataSet#findAll might be able to support this. It is not so easy to do, but possible.

So this issue contains again of two things
(i) a RFE to support CurriedClosure
(ii) a bug report for a better error message. In this case it should then check for CurriedClosure and tell it is not supported, instead of that cryptic message, which also does sound wrong. It should say at least something like "Could not find ClassNode for Class CurriedClosure" and leave MetaClass out of this.

Show

blackdrag blackdrag added a comment - 20/Mar/12 7:57 PM - edited The exception means it could not find a source file which is needed, because findAll works on the source level and for that the source needs to be re-evaluated. Now here it is Closure on which curry was used. I think DataSet#findAll might be able to support this. It is not so easy to do, but possible. So this issue contains again of two things (i) a RFE to support CurriedClosure (ii) a bug report for a better error message. In this case it should then check for CurriedClosure and tell it is not supported, instead of that cryptic message, which also does sound wrong. It should say at least something like "Could not find ClassNode for Class CurriedClosure" and leave MetaClass out of this.

Paul King made changes - 21/Mar/12 3:02 AM

Field	Original Value	New Value
Link		This issue relates to ~~GROOVY-5377~~ [ ~~GROOVY-5377~~ ]

Hide

Permalink

Paul King added a comment - 21/Mar/12 3:02 AM - edited

OK, cloned the issue and in the clone, will handle improving the error message for the normal case of the Closure not being on the classpath.

Note: the error message in the cloned issue (~~GROOVY-5377~~) isn't really meant to handle the curry case. The proposal suggests changing the error message to be something like:

DataSet unable to evaluate expression. AST not available for closure: org.codehaus.groovy.runtime.CurriedClosure. Is the source code on the classpath?

Additional work is anticipated to better handle the curry case.

Show

Paul King added a comment - 21/Mar/12 3:02 AM - edited OK, cloned the issue and in the clone, will handle improving the error message for the normal case of the Closure not being on the classpath. Note: the error message in the cloned issue ( GROOVY-5377 ) isn't really meant to handle the curry case. The proposal suggests changing the error message to be something like: DataSet unable to evaluate expression. AST not available for closure: org.codehaus.groovy.runtime.CurriedClosure. Is the source code on the classpath? Additional work is anticipated to better handle the curry case.

Hide

Permalink

Paul King added a comment - 21/Mar/12 7:00 AM

Just some further comments on DataSet. I haven't really spoken to all involved but my understanding of the history of DataSet is that it was developed as a poor man's substitute for people who didn't need a heavy weight ORM solution, e.g. trying to avoid EJBs and even hibernate. So, it was designed to be simple rather than feature rich. If you wanted something feature rich, go use hibernate (or your favourite alternative). Given these original intentions, DataSet does a reasonable job but does have some limitations.

Now, time has moved on and we have nosql and LINQ-style options available in the various languages. Groovy certainly has some wonderful options available to it courtesy of its Java heritage. However, if Groovy wants to compete with other languages, I think it needs some additional value add over the top of the Java options. I think this is certainly doable but I am not sure that DataSet (at least in its current form) will be the right way forward in the longer term. It certainly makes sense to tweak DataSet when we find obvious short-comings that can be easily fixed but for major changes I am wondering whether starting from scratch with a new offering taking into account changes in the database ecosystem might be a better use of people's time.

Having said that, I don't have any specific proposal at this time - just pointing out that perhaps we don't want to keep investing in something which is showing its age a little.

Show

Paul King added a comment - 21/Mar/12 7:00 AM Just some further comments on DataSet. I haven't really spoken to all involved but my understanding of the history of DataSet is that it was developed as a poor man's substitute for people who didn't need a heavy weight ORM solution, e.g. trying to avoid EJBs and even hibernate. So, it was designed to be simple rather than feature rich. If you wanted something feature rich, go use hibernate (or your favourite alternative). Given these original intentions, DataSet does a reasonable job but does have some limitations. Now, time has moved on and we have nosql and LINQ-style options available in the various languages. Groovy certainly has some wonderful options available to it courtesy of its Java heritage. However, if Groovy wants to compete with other languages, I think it needs some additional value add over the top of the Java options. I think this is certainly doable but I am not sure that DataSet (at least in its current form) will be the right way forward in the longer term. It certainly makes sense to tweak DataSet when we find obvious short-comings that can be easily fixed but for major changes I am wondering whether starting from scratch with a new offering taking into account changes in the database ecosystem might be a better use of people's time. Having said that, I don't have any specific proposal at this time - just pointing out that perhaps we don't want to keep investing in something which is showing its age a little.

Hide

Permalink

Russel Winder added a comment - 21/Mar/12 8:04 AM

I agree that Python's SQLAlchemy makes Groovy's DataSet look rather feature anaemic. DataSet only deals with queries and does not provide a Groovy DSL for SQL, i.e. including creating tables, inserting rows, etc. DataSet was a great start but it needs to be replaced by a Groovy equivalent of SQLAlchemy.

SQLAlchemy allows three, well four really, modes of use, simple coordination of string SQL statements, as per Groovy's Sql, the Python based DSL to replace SQL, missing from Groovy, and two ways of working with an ORM. I don't think the middle layer is a waste, it serves a valuable purpose for people who want to avoid an ORM.

Show

Russel Winder added a comment - 21/Mar/12 8:04 AM I agree that Python's SQLAlchemy makes Groovy's DataSet look rather feature anaemic. DataSet only deals with queries and does not provide a Groovy DSL for SQL, i.e. including creating tables, inserting rows, etc. DataSet was a great start but it needs to be replaced by a Groovy equivalent of SQLAlchemy. SQLAlchemy allows three, well four really, modes of use, simple coordination of string SQL statements, as per Groovy's Sql, the Python based DSL to replace SQL, missing from Groovy, and two ways of working with an ORM. I don't think the middle layer is a waste, it serves a valuable purpose for people who want to avoid an ORM.

Hide

Permalink

Paul King added a comment - 21/Mar/12 2:37 PM

Does it support the lazy evaluation? I.e. one query after several findAll/sort/reverse (or equivalent). DataSet was ahead of LINQ in that respect.

Show

Paul King added a comment - 21/Mar/12 2:37 PM Does it support the lazy evaluation? I.e. one query after several findAll/sort/reverse (or equivalent). DataSet was ahead of LINQ in that respect.

People

Assignee:

Unassigned

Reporter:

Russel Winder

Vote (0)

Watch (2)

Dates

Created:

20/Mar/12 11:15 AM

Updated:

21/Mar/12 2:37 PM