Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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;
}
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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;
}
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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;
}
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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;
}
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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.
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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.
Re: [JIRA] Commented: (CAY-811) Meaningful identity columns: user provided PK values are ignored
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.
