Re: cay-832 (enums in in exp)

From: Robert Zeigler (robert..uregumption.com)
Date: Sat Jul 28 2007 - 03:00:10 EDT

  • Next message: Andrus Adamchik: "[ANN] Cayenne 3.0M1 Released"

    Regarding lines 118 - 125, if attr is null, what should type default
    to? I really don't think that will work, honestly.
    In my own extended types, I do processing based on the type
    attribute... so does the EnumType, for that matter, since it
    determines whether to convert to int or string based on the database
    type. So, seems like the long-term "right" solution is to make sure
    that DbAttribute is something correct...

    As for debugging, I put the breakpoint in and started walking back up
    the stack.
    As predicted, it's QualifierTranslator. More specifically, the
    paramsDbType method of QueryAssemblerHelper is returning null.
    So, digging a little further, the point at which the paramsDbType
    returns null is on the unary operator check.

    Lines 286-291 of QueryAssemblerHelper:

         protected DbAttribute paramsDbType(Expression e) {
             int len = e.getOperandCount();
             // ignore unary expressions
             if (len < 2) {
                 return null;
             }

    e.getOperandCount returns 1.
    e is an instance of ASTList, which always returns 1 for the operand
    count.
    So, you get the null DbAttribute there, which is then passed into
    QueryTranslator.appendList, from there into
    QueryTranslator.appendLiteral,
    In appendLiteral, matchingObject is going to be false for any type
    that isn't a data object
    (which suggest that /any/ extended type is going to have issues with
    inExp... something I will test tomorrow).
    So, QueryAssemblerHelper.appendLiteral gets called ; again, some
    special handling is going on for data objects...
    so then we jump to QueryAssemblerHelper.appendLiteralDirect, where I
    found this comment:
    // we are hoping that when processing parameter list,
             // the correct type will be
             // guessed without looking at DbAttribute...

    Which, of course, isn't happening since the null dbattribute
    precludes the extended type stuff from having a chance to work their
    magic.

    Thoughts?

    Robert

    On Jul 27, 2007, at 7/274:18 PM , Andrus Adamchik wrote:

    > Also there may be another way to fix it by tracking down why 'attr'
    > is null in the first place (putting a breakpoint in
    > QueryAssembler.addToParamList(...) method, line 86, to see who is
    > calling it with NULL). I think the QualifierTranslator is to blame
    > here - it can't always match a value against an attribute. I expect
    > this to be a more complex fix.
    >
    > Note that the two fixes are independent from each other, and if
    > they work reliably (and won't require rewriting all translation
    > logic :-)), they both will be a good addition to Cayenne.
    >
    > Andrus
    >
    >
    > On Jul 28, 2007, at 12:07 AM, Andrus Adamchik wrote:
    >> On Jul 27, 2007, at 9:08 PM, Robert Zeigler wrote:
    >>
    >>> So, I'm interested in getting this issue resolved rather quickly,
    >>> which means I'm interested in jumping into the code.
    >>> But the implementation details of the expression parsing is not
    >>> code I've spent a lot of time looking at.
    >>> Any pointers on where to start digging? I've rummaged through
    >>> some of the expression code, some of the db adapter/sql
    >>> translator code,
    >>> and took a look at the enum converter in the java-15 subproject.
    >>> But I don't have a good sense of the big picture of how all of
    >>> the pieces fit together.
    >>> So... any pointers are appreciated (normally, I would just keep
    >>> digging on my own, but I'm in a time crunch to finish a project
    >>> right now, so the faster I can
    >>> fix this and patch it... :)
    >>>
    >>> Thanks!
    >>>
    >>> Robert
    >>
    >> Hi Robert,
    >>
    >> EnumType in java-15 subproject should be invoked by Cayenne
    >> runtime when binding the value. Judging from the behavior you
    >> observed, it is not. So here is the relevant part of the stack trace:
    >>
    >> at org.hsqldb.jdbc.jdbcPreparedStatement.setObject(Unknown Source)
    >> at org.apache.cayenne.access.trans.QueryAssembler.initStatement
    >> (QueryAssembler.java:119)
    >> at org.apache.cayenne.access.trans.QueryAssembler.createStatement
    >> (QueryAssembler.java:98)
    >>
    >>
    >> QueryAssembler.java looks like this:
    >>
    >> 118 if (attr == null) {
    >> 119 stmt.setObject(i + 1, val);
    >> 120 }
    >> 121 else {
    >> 122 int type = attr.getType();
    >> 123 int precision = attr.getPrecision();
    >> 124 adapter.bindParameter(stmt, val, i + 1, type,
    >> precision);
    >> 125 }
    >>
    >> Cayenne prematurely bailing out of correct PreparedStatement
    >> binding algorithm (that starts on line 122) and opting for a quick
    >> and dirty 'PreparedStatement.setObject(..)' on line 119 is what's
    >> causing the problem. Looking at this now, the condition check
    >> looks overly pessimistic and 'attr == null' does not justify
    >> skipping the correct flow. I think we may get rid of the "if" part
    >> and always execute "else" (just check for null on lines 122 and
    >> 123). That would require some regression testing, but logically it
    >> seems like it should work and is a better approach.
    >>
    >> Let us know if that worked for you (and didn't break the rest of
    >> Cayenne) - and we'll fix it in SVN.
    >>
    >> Thanks
    >> Andrus
    >>
    >>
    >



    This archive was generated by hypermail 2.0.0 : Sat Jul 28 2007 - 03:01:04 EDT