Community
Participate
Working Groups
When elements of a CollectionTable are modified and merged back to the EntityManager, EclipseLink issues a DELETE statement to remove the previous values from the DB. If one of the fields is a CLOB, Oracle throws an exception since you cannot have a CLOB in the WHERE clause. Doing some research, I see that this problem has been around for quite some time. The problem is that for the collection table, there is no primary key column. The JPA specification does not directly address this limitation and other JPA providers have their own implementation specific methods of dealing with it. After some digging around I found the following post from James Sutherland: https://dev.eclipse.org/mhonarc/lists/eclipselink-users/msg05047.html I followed the steps he suggested and there seems to be further bugs in EclipseLink. Since I do not see any existing tests for this scenario, I am going to assume that this never worked. Entity mappings: @Embeddable public class CollectedEntityPdo { @Lob @Column(name = "content", nullable = false) private String content; } @Entity public class ParentEntityPdo { @ElementCollection(fetch = FetchType.EAGER) @CollectionTable( joinColumns = { @JoinColumn(name = "parent_id", referencedColumnName = "id"), @JoinColumn(name = "parent_version", referencedColumnName = "version") }) private Set<CollectedEntityPdo> subs; } Exception: Exception [EclipseLink-4002] (Eclipse Persistence Services - 3.0.0.qualifier): org.eclipse.persistence.exceptions.DatabaseException Internal Exception: java.sql.SQLSyntaxErrorException: ORA-00932: inconsistent datatypes: expected - got CLOB Error Code: 932 Call: DELETE FROM ParentEntityPdo_SUBS WHERE ((((content = ?) AND (label = ?)) AND (parent_id = ?)) AND (parent_version = ?))
This scenario (oracle database, embeddedables, LOB fields) is highly likely in enterprise scenarios. Therefore I voted for this issue. A sufficient solution might be to fix the provider to not include @Lob fields for Oracle, if it is not the only field. Due to performance reasons, I would like to see providers to avoid calls to lob fields at all costs, where reasonable. As Will pointed out, you can find similar issues on stack overflow. Either they are not resolved, or contain provider-specific solutions.
Created attachment 272312 [details] master patch
Created attachment 272313 [details] 2.7 patch
Created attachment 272314 [details] 2.6_WAS patch
Comment on attachment 272312 [details] master patch - I'm missing javadoc for methods added to DatabasePlatform - you can avoid 'if' in 'shouldPrintCustomSQLForField' and directly 'return ClassConstants.BLOB.equals(type) || ClassConstants.CLOB.equals(type)' there - all 'if' checks in 'printCustomSQLForField' can be removed - it looks (..and should be documented in javadoc somewhere...) that the method is called after 'shouldPrintCustomSQLForField' which does almost the same checks; I'm also not sure if the platform needs to know the expression (first/second), it should be enough to pass DatabaseField and ExpressionSQLPrinter to this method
(In reply to Lukas Jungmann from comment #5) btw: you should also be able to replace 'ClassConstants.BLOB.equals(type)' with 'ClassConstants.BLOB == type' (same for CLOB)
Thanks for the review Lukas! > I'm missing javadoc for methods added to DatabasePlatform Your right, I did miss the javadoc that I should have added to these new methods, Ill update the patches to include proper documentation. > you can avoid 'if' in 'shouldPrintCustomSQLForField' and directly 'return ClassConstants.BLOB.equals(type) || ClassConstants.CLOB.equals(type)' there Yes, I can rewrite the 'if' check here to make the logic a bit more streamlined. > all 'if' checks in 'printCustomSQLForField' can be removed - it looks (..and should be documented in javadoc somewhere...) that the method is called after 'shouldPrintCustomSQLForField' which does almost the same checks; I added redundant checks to 'printCustomSQLForField' due to the possibility of multiple field types needing custom SQL operations in the future and the checks seemed safe enough to redo. I will document the method to only be used if the 'shouldPrintCustomSQLForField' returns a true value, but I'd like to keep the other checks. > I'm also not sure if the platform needs to know the expression (first/second), it should be enough to pass DatabaseField and ExpressionSQLPrinter to this method I am not sure how I can write the SQL as I do without passing in both the first & second Expression objects. I suppose I could just pass in 'this' CompoundExpression and obtain the first/second children to print from that Expression... Let me know if you have any other ideas. Thanks!
Created attachment 272480 [details] master patch v2
Created attachment 272481 [details] 2.7 patch v2
Created attachment 272482 [details] 2.6_WAS patch v2
I'm thinking whether solving this by platform specific expressionoperator extending functionoperator wouldn't be better/cleaner solution. WDYT?
@Lukas I had tried something very similar to this at first. The issue I was having was how to determine when the new expressionoperation applied. From my understanding, ExpressionOperators have a selector id (key) that identifies them in their map (DatasourcePlatform.platformOperators). From here I took a look at CompoundExpression.initializePlatformOperator(), however the issue is the operator is already '='. ------------- Consider the following WHERE clause LogicalExpression: Logical operator [ AND ] Logical operator [ AND ] Logical operator [ AND ] Relation operator [ = ] Field ParentEntityPdo_SUBS.content Base QUERY OBJECT Parameter ParentEntityPdo_SUBS.content Relation operator [ = ] Field ParentEntityPdo_SUBS.label Base QUERY OBJECT Parameter ParentEntityPdo_SUBS.label Relation operator [ = ] Field ParentEntityPdo_SUBS.parent_id Base QUERY OBJECT Parameter ParentEntityPdo_SUBS.parent_id Relation operator [ = ] Field ParentEntityPdo_SUBS.parent_version Base QUERY OBJECT Parameter ParentEntityPdo_SUBS.parent_version This is the expression for the following query: DELETE FROM ParentEntityPdo_SUBS WHERE ((((content = ?) AND (label = ?)) AND (parent_id = ?)) AND (parent_version = ?))) The portion that matters here is the RelationExpression: Relation operator [ = ] Field ParentEntityPdo_SUBS.content Base QUERY OBJECT Parameter ParentEntityPdo_SUBS.content NOTE: 'ParentEntityPdo_SUBS.content' is of type CLOB -------------- I first looked at ObjectBuilder.createPrimaryKeyExpression() because I figured this is where I would insert a new expressionoperation. EclipseLink creates the LogicalExpression I pasted above here by looking at the primaryKey fields and using the generic Expression.equal() operator. Unfortunately, at this point, DatasourcePlatform.platformOperators == null, so the platform operators haven't been initialized yet. I think this means the LogicalExpression has to be created with the '=' operator and not a custom operator. If there is a cleaner way to make this work, I will look into it and I am open to suggestion. Thanks for reading.
Created attachment 272654 [details] working change in ObjectBuilder I tried an experiment in ObjectBuilder.createPrimaryKeyExpression (thanks for this pointer btw!) which seems to fix the problem as well and looks a bit cleaner to me - it tries to fix the bug itself, not the buggy behaviour later... This is really quick and dirty but it should not be difficult to "hide" the 'if clob' behind some 'session.getPlatform().createExpressionFor(DatabaseField...)' call. WDYT?
Created attachment 272675 [details] master patch v3 Thanks for the advice Lukas! I went ahead and created a new patch taking into account your suggestion. Id appreciate any further thoughts and a review.
Created attachment 272676 [details] 2.7 patch v3
Created attachment 272677 [details] 2.6_WAS patch v3
Comment on attachment 272675 [details] master patch v3 yes, this is better. Looks good to me now. Thanks!
Thanks for the assistance and the review Lukas! master @ http://git.eclipse.org/c/eclipselink/eclipselink.runtime.git/commit/?id=1f1392efc2ef44fc79ea3f448b8a25ddb027c318 2.7 @ http://git.eclipse.org/c/eclipselink/eclipselink.runtime.git/commit/?h=2.7&id=ab935463df92478bc09d957243459b848ac6fecb 2.6_WAS @ http://git.eclipse.org/c/eclipselink/eclipselink.runtime.git/commit/?h=2.6_WAS&id=ce103c4bfc5cdf6c2b75875d7371a5aaf5996808
I ran the DBWS tests and I see that some tests were now failing: LoadAndSaveWithOptionsTestCases.testLoadFromInputStreamSaveDocumentToOutputStream LoadAndSaveWithOptionsTestCases.testLoadFromInputStreamWithURIAndOptionsSaveDataObjectToOutputStream LoadAndSaveWithOptionsTestCases.testLoadFromFileReaderWithURIAndOptionsStreamSaveDataObjectToWriter LoadAndSaveWithOptionsTestCases.testLoadFromAndSaveAfterDefineMultipleSchemas LoadAndSaveWithOptionsTestCases.testLoadFromInputStreamWithURIAndOptionsSaveDataObjectToStreamResult LoadAndSaveWithOptionsTestCases.testLoadFromDomSourceWithURIAndOptionsSaveDataObjectToStreamResult LoadAndSaveWithOptionsTestCases.testLoadFromSAXSourceWithURIAndOptionsSaveDataObjectToStreamResult LoadAndSaveWithOptionsTestCases.testLoadFromStreamSourceWithURIAndOptionsSaveDataObjectToStreamResult Taking a look at the error I am getting: [junit] Error: [junit] java.lang.ClassCastException: org.eclipse.persistence.oxm.platform.SAXPlatform incompatible with org.eclipse.persistence.platform.database.DatabasePlatform [junit] at org.eclipse.persistence.internal.sessions.DatabaseSessionImpl.getPlatform(DatabaseSessionImpl.java:509) [junit] at org.eclipse.persistence.internal.descriptors.ObjectBuilder.createPrimaryKeyExpression(ObjectBuilder.java:2914) [junit] at org.eclipse.persistence.internal.descriptors.ObjectBuilder.initializePrimaryKey(ObjectBuilder.java:3881) [junit] at org.eclipse.persistence.internal.oxm.XMLObjectBuilder.initialize(XMLObjectBuilder.java:615) [junit] at org.eclipse.persistence.internal.oxm.TreeObjectBuilder.initialize(TreeObjectBuilder.java:94) [junit] at org.eclipse.persistence.oxm.XMLDescriptor.initialize(XMLDescriptor.java:774) [junit] at org.eclipse.persistence.sdo.helper.delegates.SDOXMLHelperDelegate.initializeDescriptor(SDOXMLHelperDelegate.java:648) It seems my change introduced a failure in this test bucket. I am working on fixing this.
Tests in SDO are failing due to this change, reopening to fix
Taking a look at the failure, it seems my change was not taking into account org.eclipse.persistence.oxm.platform.SAXPlatform, which does not extend DatabasePlatform, but instead extends DatasourcePlatform. I created a new patch that fixes this issue. I ran this fix against the sdo testing and they passed without failure.
Created attachment 272793 [details] master/2.7 patch v3 pt2 Patch to fix SDO failing tests
Created attachment 272794 [details] 2.6_WAS patch v3 pt2
Comment on attachment 272793 [details] master/2.7 patch v3 pt2 I'm fine with this. Alternative would be to "fallback" to the original in ObjectBuilder if platform. createExpressionFor returns null... But I'll leave that up to you..
master @ http://git.eclipse.org/c/eclipselink/eclipselink.runtime.git/commit/?id=dffd2942f3dcc2e34027c4ef9ca9f2e0183b3089 2.7 @ http://git.eclipse.org/c/eclipselink/eclipselink.runtime.git/commit/?h=2.7&id=72060bb559daa2846a21757cae6ec9179735c3ab 2.6_WAS @ http://git.eclipse.org/c/eclipselink/eclipselink.runtime.git/commit/?h=2.6_WAS&id=b0ea0c5eb9b7886a4b2f7cbd02ba2e22181bfcc7
Master: https://github.com/eclipse-ee4j/eclipselink/pull/430
Disregard previous comment, wrong bug.
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink
