|
[
Permlink
| « Hide
]
Stan Bradbury added a comment - 30/Nov/07 10:09 PM
Delete database blobInsertDB before rerunning the program.
In an UpdateResultSet row there is a column for each "before" value and another column for the "after" value. When a trigger is present all columns are pulled in as update columns even if they are not updated, so each column has a duplicate column values in the row. In this case, the BLOB column was not updated, so the before value and the after value point to the same stream.
The before value gets materialized in getNextRowCore-> DMLWriteResultSet.getNextRowCore(), but the after value does not get updated at that time. Later in collectAffectedRows we try to materialize the after value by calling objectifyStream on the after columns, but because the stream was already read, we get the EOFException. It seems there are a few options. 1) Try to fix DERBY-1482 so that the non-updated columns don't even show up in the UpdateResultSet or at least we don't have to materialize them (hard?). 2) Try to make sure that duplicate columns in the row get updated when the stream is originally loaded. I am looking at this, but am not sure exactly the best place in the code to copy the stream value. 3) Something more clever. Ideas? I think long term a fix along the lines of 1 would be great, and probably more appropriate for a future feature release. It seems like trigger access code could be optimized if we had new complile time code which gathered all the following:
1) which columns are ever accessed for both before and after values in the trigger. 2) in case of after values, which columns actually may change as part of the update (maybe we have this - i don't know). Then execution code could be changed to not have to deal with any columns that the trigger need ever access. But if the goal is to get a fix for existing release, I think something along the lines of your #2 suggestion would be fine. I assume the problem is that when we "copy" the datavaluedescriptor to the "after" column we do a "shallow" copy. Then subsequently we drain the stream for the before rows, leaving the shallow copy thinking it has a stream when it really does not. So if you can get the objectify to happen on the before column before the copy things should work right. Here is an initial patch attempted to resolve the issue. I am running tests now. It changes DMLWriteResultSet.objectifyStreams() to check for duplicate streams and update them with a cloned DataValueDescriptor of the value that we objectified. I wish I had been able to have more context and been able to calculate the exact offset to update. As it is, when a stream is objectified, we search all the other columns to see if there are duplicates. It has the advantage of working, but seems like unnecessary work.
Anyway, just throwing this out there while the tests runs to see if I am on the right track. Kathey Kathey, I was wondering if the new test testClobInTriggerTable is supposed to be creating a CLOB column rather than BLOB. Because it looks like BLOB is already getting tested in the other new test testBlobInTriggerTable.
Thanks Mamta,
I must have gotten distracted mid cut and paste. Here is the revised patch. tests ran ok except an OutOfMemory exception in DatabaseMetaDataTest which looks unrelated. I ran the test individually and it ran ok. I am rerunning the whole suite to make sure this test did not somehow have a negative impact on the full run with its 15MB Blob.
The test fixtures do not close the statements they create. If they created the statement using the utility methods instead then the statements would be closed automatically.
Thanks Dan, attaching v3 patch.
Kathey, can we add a test where there is an update trigger defined on LOB column and then write a test that updates theLOB to make sure that the LOBs get updated correctly with their befor and after values in trigger code. I am not sure if there are any restrictions on defining triggers which deal with LOB columns.
Also, can we have a test on a table that has mutliple LOB columns to make sure that DMLWriteResultSet.objectifyStreams() works correctly for more than one LOB column in a table? The statements in testBlobSize()/testClobSize() are not closed.
I think if you made these methods non-static and did not pass in the connection, then it becomes clearer for readers that these methods are using the default connection, otherwise one has to assume that they are not. Attached is derby-3238_diffv4.txt which contains the tests Mamta suggested as well as cleaning up the testBlobInTriggerTable() and testClobInTriggerTable() tests. Hopefully I am only using utility methods now so don't need to close the statements.
Kathey thanks for adding additional tests, Kathey. The changes look good to me.
testBlobInTriggerTable(int blobSize) (and the clob version) do the following ...
Connection conn = getConnection(); .... conn.commit(); .... conn.commit(); etc. A commit() utility method exists that would remove the need for getConnection(). While the functionality is the same, the use of the utility methods allow a reader to instantly see that this is a fixture (code) working on the default connection. Use of a local conn variable means a reader has to figure out if this a multi-onnection test or not. Thus the utility methods try to remove the amount of code needed for a test (to make the asserts stand out), and to make the simple typical single user test easier to understand. Cleanup is not required for committing the patch. The problem on 10.1 looks different than on 10.2,10.3, and 10.4
It fails at a slightly larger blob and error is when we try to objectify the before column not the after column. Also in 10.1 clobs work fine without the fix. I will backport the fix to 10.3 and 10.2 and investigate the 10.1 problem. Not sure yet if I should file a separate bug for that. >java blob_insert2 Testing blob of size=1024 . . Now executing update to fire the trigger PASSED Testing blob of size=16384 . . Now executing update to fire the trigger PASSED Testing blob of size=32658 . . Now executing update to fire the trigger PASSED Testing blob of size=32659 . . Now executing update to fire the trigger PASSED Testing blob of size=32767 . . Now executing update to fire the trigger Error! SQL Exception: An IOException was thrown when reading a 'BLOB' from an InputStream. ERROR XCL30: An IOException was thrown when reading a 'BLOB' from an InputStream. at org.apache.derby.iapi.error.StandardException.newException(StandardException.java:315) at org.apache.derby.iapi.types.SQLBinary.getValue(SQLBinary.java:214) at org.apache.derby.iapi.types.SQLBinary.loadStream(SQLBinary.java:529) at org.apache.derby.impl.sql.execute.DMLWriteResultSet.objectifyStreams(DMLWriteResultSet.java:151) at org.apache.derby.impl.sql.execute.DMLWriteResultSet.getNextRowCore(DMLWriteResultSet.java:132) at org.apache.derby.impl.sql.execute.UpdateResultSet.collectAffectedRows(UpdateResultSet.java:450) at org.apache.derby.impl.sql.execute.UpdateResultSet.open(UpdateResultSet.java:276) at org.apache.derby.impl.sql.GenericPreparedStatement.execute(GenericPreparedStatement.java:379) at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(EmbedStatement.java:1123) at org.apache.derby.impl.jdbc.EmbedStatement.execute(EmbedStatement.java:529) at org.apache.derby.impl.jdbc.EmbedStatement.executeUpdate(EmbedStatement.java:165) at blob_insert2.testBlob(blob_insert2.java:114) at blob_insert2.main(blob_insert2.java:65) [ fixed in trunk, 10.3, and 10.2. Investigating 10.1
Attached is a patch for version 10.1 for this issue. In addition to the trunk patch, I had to change SQLBlob.setWidth to not materialize the lob if the length is unknown. This change was checked into trunk with revision 437980 as part of
The extra problem in 10.1 was that the "after" column was getting materialized with normalizeRow which called setWidth, so we were in a state where the before column was still pointing to the stream, even though the after column had been materialized. The error was occurring when we tried to objectify the before column. Eliminating the materialization with setWidth means the stream has not been read when we call objectifyStream. That in combination with the trunk patch allows the test case to pass. The reason the clob case was passing without the patch on 10.1 is that the before and after columns were pointing to the same DataValueDescriptor. That, I did not change and I am not sure what problems that might cause, but for this case it worked to our advantage. Tests are running on 10.1. Kathey Catching up on Closing my reported issues. Thanks to Dyre for the workflow reminder today.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||