FAQ
Tore,

let me take this to the list, as it would be much easier to follow
the discussion compared to Jira comments. First, this is indeed a
separate issue from CAY-811 main problem, as it has nothing to do
with database generated PK.

Now that you posted a patch below I remember why it was done this way
- to support PK generation for primitive int attributes, required by
JPA (and our POJO feature). Now I guess we'll need to figure out a
way in DataDomainInsertBucket to tell apart meaningful primitives vs.
java.lang.Number subclasses. Otherwise the patch below would break JPA.

Andrus

On Aug 2, 2007, at 2:36 PM, Tore Halset (JIRA) wrote:


[ https://issues.apache.org/cayenne/browse/CAY-811?
page=com.atlassian.jira.plugin.system.issuetabpanels:comment-
tabpanel#action_12430 ]

Tore Halset commented on CAY-811:
---------------------------------

This small patch fixes my issue (int meaningful primary key *not*
marked as generated). Is it the same as your 1) in the description
above? I would love to commit it, but I do not know why the special
handling of a Number with the intValue 0 was added?

Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
apache/cayenne/access/DataDomainInsertBucket.java
===================================================================
--- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
cayenne/access/DataDomainInsertBucket.java (revision 562071)
+++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
cayenne/access/DataDomainInsertBucket.java (working copy)
@@ -144,11 +144,8 @@
.readPropertyDirectly(object);

if (value != null) {
- // treat numeric zero values as nulls
requiring generation
- if (!(value instanceof Number && ((Number)
value).intValue() == 0)) {
- idMap.put(dbAttrName, value);
- continue;
- }
+ idMap.put(dbAttrName, value);
+ continue;
}
}


Meaningful identity columns: user provided PK values are ignored
----------------------------------------------------------------

Key: CAY-811
URL: https://issues.apache.org/cayenne/browse/CAY-811
Project: Cayenne
Issue Type: Bug
Components: Cayenne Core Library
Affects Versions: 1.2 [STABLE], 2.0 [STABLE], 3.0
Reporter: Andrus Adamchik
Assignee: Andrus Adamchik
Priority: Minor
Fix For: 3.0


I found it when testing on 3.0, although I suspect this is a
problem on 2.0 and 1.2 as well. When a meaningful PK is set by the
user and is also mapped as a DB-generated PK, Cayenne incorrectly
overrides user value. There are two places where this must be fixed:
1. DataDomainInsertBucket (I will commit the fix shortly)
2. InsertBatchQueryBuilder (this is a bit more hairy - as the
batch syntax will be affected depending on whether user provided
a value or not)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Search Discussions

  • Tore Halset at Aug 2, 2007 at 12:44 pm

    On Aug 2, 2007, at 13:58 , Andrus Adamchik wrote:

    let me take this to the list, as it would be much easier to follow
    the discussion compared to Jira comments. First, this is indeed a
    separate issue from CAY-811 main problem, as it has nothing to do
    with database generated PK.
    Okay, I reopened CAY-835.
    Now that you posted a patch below I remember why it was done this
    way - to support PK generation for primitive int attributes,
    required by JPA (and our POJO feature). Now I guess we'll need to
    figure out a way in DataDomainInsertBucket to tell apart meaningful
    primitives vs. java.lang.Number subclasses.
    Handling null as 0 can probably lead to other problems as well.
    Perhaps 'NullNumber extends Number' could be used? Or creating a
    (empty) Null-interface and then subclass Integer, Long and so on?
    Btw: Øyvind Harboe has lots of good thoughts on null handling.
    Otherwise the patch below would break JPA.
    Or unbreak cayenne classic :)

    Regards,
    - Tore.
    Andrus

    On Aug 2, 2007, at 2:36 PM, Tore Halset (JIRA) wrote:


    [ https://issues.apache.org/cayenne/browse/CAY-811?
    page=com.atlassian.jira.plugin.system.issuetabpanels:comment-
    tabpanel#action_12430 ]

    Tore Halset commented on CAY-811:
    ---------------------------------

    This small patch fixes my issue (int meaningful primary key *not*
    marked as generated). Is it the same as your 1) in the description
    above? I would love to commit it, but I do not know why the
    special handling of a Number with the intValue 0 was added?

    Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
    apache/cayenne/access/DataDomainInsertBucket.java
    ===================================================================
    --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (revision 562071)
    +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (working copy)
    @@ -144,11 +144,8 @@
    .readPropertyDirectly(object);

    if (value != null) {
    - // treat numeric zero values as nulls
    requiring generation
    - if (!(value instanceof Number &&
    ((Number) value).intValue() == 0)) {
    - idMap.put(dbAttrName, value);
    - continue;
    - }
    + idMap.put(dbAttrName, value);
    + continue;
    }
    }


    Meaningful identity columns: user provided PK values are ignored
    ----------------------------------------------------------------

    Key: CAY-811
    URL: https://issues.apache.org/cayenne/browse/
    CAY-811
    Project: Cayenne
    Issue Type: Bug
    Components: Cayenne Core Library
    Affects Versions: 1.2 [STABLE], 2.0 [STABLE], 3.0
    Reporter: Andrus Adamchik
    Assignee: Andrus Adamchik
    Priority: Minor
    Fix For: 3.0


    I found it when testing on 3.0, although I suspect this is a
    problem on 2.0 and 1.2 as well. When a meaningful PK is set by
    the user and is also mapped as a DB-generated PK, Cayenne
    incorrectly overrides user value. There are two places where this
    must be fixed:
    1. DataDomainInsertBucket (I will commit the fix shortly)
    2. InsertBatchQueryBuilder (this is a bit more hairy - as the
    batch syntax will be affected depending on whether user provided
    a value or not)
    --
    This message is automatically generated by JIRA.
    -
    You can reply to this email to add a comment to the issue online.
  • Andrus Adamchik at Aug 2, 2007 at 12:53 pm

    On Aug 2, 2007, at 3:43 PM, Tore Halset wrote:

    Handling null as 0 can probably lead to other problems as well.
    I guess that would be "handling zero as null". As far as I am aware
    this is the only place where we stretched that a bit. I need some
    more time to look at the code - maybe we can distinguish between the
    two cases by looking at the ObjAttribute (which should have an
    indication of whether the value is primitive).
    Øyvind Harboe has lots of good thoughts on null handling.
    Please share :-) (or was it already and I missed the relevant emails?)

    Andrus
  • Tore Halset at Aug 2, 2007 at 12:53 pm

    On Aug 2, 2007, at 14:43 , Tore Halset wrote:

    Handling null as 0 can probably lead to other problems as well.
    Perhaps 'NullNumber extends Number' could be used? Or creating a
    (empty) Null-interface and then subclass Integer, Long and so on?
    The last one would not work as Integer is final.

    - Tore.
  • Andrus Adamchik at Aug 2, 2007 at 12:59 pm

    On Aug 2, 2007, at 3:52 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:43 , Tore Halset wrote:

    Handling null as 0 can probably lead to other problems as well.
    Perhaps 'NullNumber extends Number' could be used? Or creating a
    (empty) Null-interface and then subclass Integer, Long and so on?
    The last one would not work as Integer is final.

    - Tore.
    I didn't quite understand the proposed solution (aside from the
    "final" thing). What are we going to use NullNunber for? Could maybe
    post some code examples?

    Andrus
  • Tore Halset at Aug 2, 2007 at 3:36 pm

    On Aug 2, 2007, at 14:59 , Andrus Adamchik wrote:
    On Aug 2, 2007, at 3:52 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:43 , Tore Halset wrote:

    Handling null as 0 can probably lead to other problems as well.
    Perhaps 'NullNumber extends Number' could be used? Or creating a
    (empty) Null-interface and then subclass Integer, Long and so on?
    The last one would not work as Integer is final.

    - Tore.
    I didn't quite understand the proposed solution (aside from the
    "final" thing). What are we going to use NullNunber for? Could
    maybe post some code examples?

    A better name would be UnsetPrimitiveNumber extending Number and
    return 0 for all the methods. That way it would be the almost same as
    new Integer(0), but could be tested for its type.

    I have digged a bit down in the POJO code and it looks like this
    approach will not work. Using reflection on a POJO, java will report
    the same for an unset int as an int set to 0. So (at least from a
    reflection point of view) it is the same.

    Could we use the DbAttribute.isGenerated flag to determine if the new
    Integer(0)-value should be handled? Attached is a patch that explain
    this variant. It looks like this variant passes all the junit tests
    and also fixes my problem.

    - Tore.
    Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java
    ===================================================================
    --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (revision 562134)
    +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (working copy)
    @@ -144,8 +144,12 @@
    .readPropertyDirectly(object);
    if (value != null) {
    - // treat numeric zero values as nulls
    requiring generation
    - if (!(value instanceof Number && ((Number)
    value).intValue() == 0)) {
    + // POJO/JPA with generated key mapped as a
    primitive type will
    + // have a Number with value 0 for a unset value
    + if (!(supportsGeneratedKeys
    + && dbAttr.isGenerated()
    + && (value instanceof Number) &&
    ((Number) value)
    + .intValue() == 0)) {
    idMap.put(dbAttrName, value);
    continue;
    }
  • Andrus Adamchik at Aug 2, 2007 at 3:43 pm
    I see... The problem is that 'isGenerated' only applies to 'Db
    Generated' PK strategy and does not apply to 'Default'. So maybe
    leave the zero rule for primitives, and check the ObjAttribute (don't
    know if it is easily available in that place without looking at the
    code?) for whether the value is mapped as numeric primitive or
    numeric object?

    Andrus

    On Aug 2, 2007, at 6:35 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:59 , Andrus Adamchik wrote:
    On Aug 2, 2007, at 3:52 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:43 , Tore Halset wrote:

    Handling null as 0 can probably lead to other problems as well.
    Perhaps 'NullNumber extends Number' could be used? Or creating a
    (empty) Null-interface and then subclass Integer, Long and so on?
    The last one would not work as Integer is final.

    - Tore.
    I didn't quite understand the proposed solution (aside from the
    "final" thing). What are we going to use NullNunber for? Could
    maybe post some code examples?

    A better name would be UnsetPrimitiveNumber extending Number and
    return 0 for all the methods. That way it would be the almost same
    as new Integer(0), but could be tested for its type.

    I have digged a bit down in the POJO code and it looks like this
    approach will not work. Using reflection on a POJO, java will
    report the same for an unset int as an int set to 0. So (at least
    from a reflection point of view) it is the same.

    Could we use the DbAttribute.isGenerated flag to determine if the
    new Integer(0)-value should be handled? Attached is a patch that
    explain this variant. It looks like this variant passes all the
    junit tests and also fixes my problem.

    - Tore.
    Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
    apache/cayenne/access/DataDomainInsertBucket.java
    ===================================================================
    --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (revision 562134)
    +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (working copy)
    @@ -144,8 +144,12 @@
    .readPropertyDirectly(object);
    if (value != null) {
    - // treat numeric zero values as nulls
    requiring generation
    - if (!(value instanceof Number && ((Number)
    value).intValue() == 0)) {
    + // POJO/JPA with generated key mapped as a
    primitive type will
    + // have a Number with value 0 for a unset
    value
    + if (!(supportsGeneratedKeys
    + && dbAttr.isGenerated()
    + && (value instanceof Number) &&
    ((Number) value)
    + .intValue() == 0)) {
    idMap.put(dbAttrName, value);
    continue;
    }

  • Andrus Adamchik at Aug 2, 2007 at 6:42 pm
    Finally had a chance to look at the code (instead of suggesting
    random things :-)). I think a patch like that would work (in fact I
    tested it with POJO itests and it seems to do the right thing. If
    there are no objections I can commit it tomorrow.

    Andrus


    Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java
    ===================================================================
    --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (revision 560823)
    +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (working copy)
    @@ -144,8 +144,15 @@
    .readPropertyDirectly(object);
    if (value != null) {
    - // treat numeric zero values as nulls
    requiring generation
    - if (!(value instanceof Number && ((Number)
    value).intValue() == 0)) {
    + Class javaClass = objAttr.getJavaClass();
    + if (javaClass.isPrimitive()
    + && value instanceof Number
    + && ((Number) value).intValue() == 0) {
    + // primitive 0 has to be treated as
    NULL, or otherwise we
    + // can't generate PK for POJO's
    + }
    + else {
    +
    idMap.put(dbAttrName, value);
    continue;
    }


    On Aug 2, 2007, at 6:42 PM, Andrus Adamchik wrote:

    I see... The problem is that 'isGenerated' only applies to 'Db
    Generated' PK strategy and does not apply to 'Default'. So maybe
    leave the zero rule for primitives, and check the ObjAttribute
    (don't know if it is easily available in that place without looking
    at the code?) for whether the value is mapped as numeric primitive
    or numeric object?

    Andrus

    On Aug 2, 2007, at 6:35 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:59 , Andrus Adamchik wrote:
    On Aug 2, 2007, at 3:52 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:43 , Tore Halset wrote:

    Handling null as 0 can probably lead to other problems as well.
    Perhaps 'NullNumber extends Number' could be used? Or creating
    a (empty) Null-interface and then subclass Integer, Long and so
    on?
    The last one would not work as Integer is final.

    - Tore.
    I didn't quite understand the proposed solution (aside from the
    "final" thing). What are we going to use NullNunber for? Could
    maybe post some code examples?

    A better name would be UnsetPrimitiveNumber extending Number and
    return 0 for all the methods. That way it would be the almost same
    as new Integer(0), but could be tested for its type.

    I have digged a bit down in the POJO code and it looks like this
    approach will not work. Using reflection on a POJO, java will
    report the same for an unset int as an int set to 0. So (at least
    from a reflection point of view) it is the same.

    Could we use the DbAttribute.isGenerated flag to determine if the
    new Integer(0)-value should be handled? Attached is a patch that
    explain this variant. It looks like this variant passes all the
    junit tests and also fixes my problem.

    - Tore.
    Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
    apache/cayenne/access/DataDomainInsertBucket.java
    ===================================================================
    --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (revision 562134)
    +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (working copy)
    @@ -144,8 +144,12 @@
    .readPropertyDirectly(object);
    if (value != null) {
    - // treat numeric zero values as nulls
    requiring generation
    - if (!(value instanceof Number &&
    ((Number) value).intValue() == 0)) {
    + // POJO/JPA with generated key mapped as
    a primitive type will
    + // have a Number with value 0 for a unset
    value
    + if (!(supportsGeneratedKeys
    + && dbAttr.isGenerated()
    + && (value instanceof Number) &&
    ((Number) value)
    + .intValue() == 0)) {
    idMap.put(dbAttrName, value);
    continue;
    }

  • Tore Halset at Aug 3, 2007 at 7:04 am
    Hello.

    Thank you for looking into the issue. That patch is ok for me.

    Regards,
    - Tore.
    On Aug 2, 2007, at 20:41 , Andrus Adamchik wrote:

    Finally had a chance to look at the code (instead of suggesting
    random things :-)). I think a patch like that would work (in fact I
    tested it with POJO itests and it seems to do the right thing. If
    there are no objections I can commit it tomorrow.

    Andrus


    Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
    apache/cayenne/access/DataDomainInsertBucket.java
    ===================================================================
    --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (revision 560823)
    +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (working copy)
    @@ -144,8 +144,15 @@
    .readPropertyDirectly(object);
    if (value != null) {
    - // treat numeric zero values as nulls
    requiring generation
    - if (!(value instanceof Number && ((Number)
    value).intValue() == 0)) {
    + Class javaClass = objAttr.getJavaClass();
    + if (javaClass.isPrimitive()
    + && value instanceof Number
    + && ((Number) value).intValue() ==
    0) {
    + // primitive 0 has to be treated as
    NULL, or otherwise we
    + // can't generate PK for POJO's
    + }
    + else {
    +
    idMap.put(dbAttrName, value);
    continue;
    }


    On Aug 2, 2007, at 6:42 PM, Andrus Adamchik wrote:

    I see... The problem is that 'isGenerated' only applies to 'Db
    Generated' PK strategy and does not apply to 'Default'. So maybe
    leave the zero rule for primitives, and check the ObjAttribute
    (don't know if it is easily available in that place without
    looking at the code?) for whether the value is mapped as numeric
    primitive or numeric object?

    Andrus

    On Aug 2, 2007, at 6:35 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:59 , Andrus Adamchik wrote:
    On Aug 2, 2007, at 3:52 PM, Tore Halset wrote:
    On Aug 2, 2007, at 14:43 , Tore Halset wrote:

    Handling null as 0 can probably lead to other problems as
    well. Perhaps 'NullNumber extends Number' could be used? Or
    creating a (empty) Null-interface and then subclass Integer,
    Long and so on?
    The last one would not work as Integer is final.

    - Tore.
    I didn't quite understand the proposed solution (aside from the
    "final" thing). What are we going to use NullNunber for? Could
    maybe post some code examples?

    A better name would be UnsetPrimitiveNumber extending Number and
    return 0 for all the methods. That way it would be the almost
    same as new Integer(0), but could be tested for its type.

    I have digged a bit down in the POJO code and it looks like this
    approach will not work. Using reflection on a POJO, java will
    report the same for an unset int as an int set to 0. So (at least
    from a reflection point of view) it is the same.

    Could we use the DbAttribute.isGenerated flag to determine if the
    new Integer(0)-value should be handled? Attached is a patch that
    explain this variant. It looks like this variant passes all the
    junit tests and also fixes my problem.

    - Tore.
    Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
    apache/cayenne/access/DataDomainInsertBucket.java
    ===================================================================
    --- framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (revision 562134)
    +++ framework/cayenne-jdk1.4-unpublished/src/main/java/org/apache/
    cayenne/access/DataDomainInsertBucket.java (working copy)
    @@ -144,8 +144,12 @@
    .readPropertyDirectly(object);
    if (value != null) {
    - // treat numeric zero values as nulls
    requiring generation
    - if (!(value instanceof Number &&
    ((Number) value).intValue() == 0)) {
    + // POJO/JPA with generated key mapped as
    a primitive type will
    + // have a Number with value 0 for a
    unset value
    + if (!(supportsGeneratedKeys
    + && dbAttr.isGenerated()
    + && (value instanceof Number) &&
    ((Number) value)
    + .intValue() == 0)) {
    idMap.put(dbAttrName, value);
    continue;
    }

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupdev @
categoriescayenne
postedAug 2, '07 at 11:59a
activeAug 3, '07 at 7:04a
posts9
users2
websitecayenne.apache.org

2 users in discussion

Andrus Adamchik: 5 posts Tore Halset: 4 posts

People

Translate

site design / logo © 2021 Grokbase