This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 206233 - Improve BLOB/CLOB Handling
Summary: Improve BLOB/CLOB Handling
Status: RESOLVED FIXED
Alias: None
Product: db
Classification: Unclassified
Component: Show Data (show other bugs)
Version: 7.0.1
Hardware: PC Linux
: P3 normal (vote)
Assignee: Jiri Rechtacek
URL:
Keywords: NETFIX
: 209834 210156 (view as bug list)
Depends on:
Blocks: 189020
  Show dependency tree
 
Reported: 2011-12-11 14:19 UTC by matthias42
Modified: 2012-04-05 11:44 UTC (History)
2 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
preliminary patch for first review (72.48 KB, patch)
2011-12-11 14:19 UTC, matthias42
Details | Diff
patch (additionally to the first patch) (15.62 KB, patch)
2011-12-12 20:58 UTC, matthias42
Details | Diff
patch (additionally to the first patch) (16.69 KB, patch)
2011-12-12 20:58 UTC, matthias42
Details | Diff
break massive celleditor class file into smaller files (70.52 KB, patch)
2011-12-16 20:15 UTC, matthias42
Details | Diff
fix celleditor split 1 (847 bytes, patch)
2011-12-16 20:16 UTC, matthias42
Details | Diff
fix celleditor split 2 (3.96 KB, patch)
2011-12-16 20:17 UTC, matthias42
Details | Diff
add progressmonitor to the blob/clob loading (17.90 KB, patch)
2011-12-16 20:19 UTC, matthias42
Details | Diff
initial patch adding file backed pseudoblobs and changes using the new structures (96.07 KB, patch)
2011-12-18 18:58 UTC, matthias42
Details | Diff
Break massive CellEditor Class apart and move currently non-public classes into seperate package and files (74.06 KB, patch)
2011-12-18 18:59 UTC, matthias42
Details | Diff
Add progress dialog for (estimated) long transfers when saving/loading BLOB/CLOB data (17.46 KB, patch)
2011-12-18 19:00 UTC, matthias42
Details | Diff
Add simple editor for CLOBs (based on string editor) (8.15 KB, patch)
2011-12-18 19:00 UTC, matthias42
Details | Diff
Make encondings more robust (2.51 KB, patch)
2012-02-10 22:24 UTC, matthias42
Details | Diff
fix null pointer exceptions at various places (11.02 KB, patch)
2012-02-18 21:26 UTC, matthias42
Details | Diff
stacktrace (864 bytes, text/plain)
2012-04-03 11:53 UTC, schkovich
Details
fix null pointer exceptions at various places (6.26 KB, patch)
2012-04-03 20:41 UTC, matthias42
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description matthias42 2011-12-11 14:19:36 UTC
Created attachment 114026 [details]
preliminary patch for first review

The DataView handles large binary objects (LOBs) loading a subset of the whole LOB (2000 chars for CLOBs, 2000bytes for BLOBs). The resulting value for blobs is then Hexencoded and presented as a string.

I doubt the usefulness of this and created a prelimenary patch, which is a first step towards better handling of blobs/clobs.

The idea: Try to keep the data intact by loading the database value completly into a pseudo LOB. This object could have been a Serial(C/B)lob from the rowset family of classes. But these objects are held in memory and I think the restrictions above were placed in the code to limit memory pressure. So this patch basicly creates a temporary file per blob/clob and exposed this temporary file via the Clob/Blob Interfaces.

My first idea was to keep the resultset open and fetch the real database blobs/clobs on-the-fly, but this might pose problems with regard to the stability of the jdbc handling of the drivers (blobs might be closed, not readable after reading over the line in the resultset, or the connection might be timed out). What is more an open blob might place a row lock in the database and so prevent normal operation on a production database.

Things I want to improve for a final version:

* Add immutable pseudo blobs backed by real files to speedup handling large files (currently files are copied before an update/insert is issued)
* Create tests for the Blob/Clob implementations
* Add feedback when adding large files

Please have a look at the patch and give me feedback whether or not this can be applied with the improvements described above.
Comment 1 matthias42 2011-12-11 14:21:58 UTC
Ah! I missed one improvement: The ability to change CLOB values in an editor window/dialog, as it is currently possible (most likely with a limit to the max. editable length).
Comment 2 matthias42 2011-12-12 20:58:09 UTC
Created attachment 114068 [details]
patch (additionally to the first patch)

Add unittests for filebacked blobs and fix the filebacked blobs on the way.
Comment 3 matthias42 2011-12-12 20:58:59 UTC
Created attachment 114069 [details]
patch (additionally to the first patch)

Add unittests for filebacked clobs and fix the filebacked clobs on the way.
Comment 4 matthias42 2011-12-16 20:15:37 UTC
Created attachment 114279 [details]
break massive celleditor class file into smaller files
Comment 5 matthias42 2011-12-16 20:16:28 UTC
Created attachment 114280 [details]
fix celleditor split 1
Comment 6 matthias42 2011-12-16 20:17:03 UTC
Created attachment 114281 [details]
fix celleditor split 2
Comment 7 matthias42 2011-12-16 20:19:01 UTC
Created attachment 114282 [details]
add progressmonitor to the blob/clob loading
Comment 8 matthias42 2011-12-18 18:58:49 UTC
Created attachment 114287 [details]
initial patch adding file backed pseudoblobs and changes using the new structures
Comment 9 matthias42 2011-12-18 18:59:28 UTC
Created attachment 114288 [details]
Break massive CellEditor Class apart and move currently non-public classes into seperate package and files
Comment 10 matthias42 2011-12-18 19:00:00 UTC
Created attachment 114289 [details]
Add progress dialog for (estimated) long transfers when saving/loading BLOB/CLOB data
Comment 11 matthias42 2011-12-18 19:00:22 UTC
Created attachment 114290 [details]
Add simple editor for CLOBs (based on string editor)
Comment 12 Jiri Rechtacek 2012-01-03 08:33:47 UTC
I'm sorry Matthias I didn't look on your patches sooner I'm have been busy on other IDE components for what I'm responsible too. I'll look on it today. Thanks for your patience.
Comment 13 Jiri Rechtacek 2012-02-10 12:51:28 UTC
I tried applied your patch but db.dataview's unit tests failed. See:

Testcase: testSetString_long_String(org.netbeans.modules.db.dataview.util.FileBackedClobTest):	FAILED
expected:<[]Tes> but was:<[t]Tes>
junit.framework.AssertionFailedError: expected:<[]Tes> but was:<[t]Tes>
	at org.netbeans.modules.db.dataview.util.FileBackedClobTest.testSetString_long_String(FileBackedClobTest.java:212)


Testcase: testSetString_4args(org.netbeans.modules.db.dataview.util.FileBackedClobTest):	FAILED
expected:<[]Tes> but was:<[t]Tes>
junit.framework.AssertionFailedError: expected:<[]Tes> but was:<[t]Tes>
	at org.netbeans.modules.db.dataview.util.FileBackedClobTest.testSetString_4args(FileBackedClobTest.java:234)


Testcase: testSetCharacterStream(org.netbeans.modules.db.dataview.util.FileBackedClobTest):	FAILED
expected:<[]Tes> but was:<[t]Tes>
junit.framework.AssertionFailedError: expected:<[]Tes> but was:<[t]Tes>
	at org.netbeans.modules.db.dataview.util.FileBackedClobTest.testSetCharacterStream(FileBackedClobTest.java:254)


Test org.netbeans.modules.db.dataview.util.FileBackedClobTest FAILED
Comment 14 matthias42 2012-02-10 22:24:02 UTC
Created attachment 115612 [details]
Make encondings more robust

Hey - first: Thanks, you flooded my inbox with accepted patches!

(In reply to comment #13)
> I tried applied your patch but db.dataview's unit tests failed. 

I checked again and could not reproduce the error (tried forcing different source encoding and enviroment variables). I see two potential error sources:

1. Testdata could be corrupted => the attached patch encodes the testdata to unicode escape => should always work
2. I used UTF32 for backing storage of the character data. According to this: http://docs.oracle.com/javase/6/docs/technotes/guides/intl/encoding.doc.html - there could be a BOM, that would break the internal logic. So the patch enforces UTF_32BE without BOM.

Could you please apply the patch on top of "initial patch adding file backed pseudoblobs and changes using the new structures". And run the test again?

If it fails please debug that Testfile with a breakpoint in the Testmethod "testSetString_long_String" at the first assert. Then please check your temp-directory there you should find two files named "netbeans-db-blob...". It would be helpful to have that file.

Could you also please provide info about your test platform - maybe I can create a test installation for me (i run this on ubuntu 11.10 with openjdk).

Thanks Matthias
Comment 15 Jiri Rechtacek 2012-02-14 20:14:08 UTC
(In reply to comment #14)
> Created attachment 115612 [details]
> Make encondings more robust
> 
> Hey - first: Thanks, you flooded my inbox with accepted patches!
> 
> (In reply to comment #13)
> > I tried applied your patch but db.dataview's unit tests failed. 
> 
> I checked again and could not reproduce the error (tried forcing different
> source encoding and enviroment variables). I see two potential error sources:
> 
> 1. Testdata could be corrupted => the attached patch encodes the testdata to
> unicode escape => should always work
> 2. I used UTF32 for backing storage of the character data. According to this:
> http://docs.oracle.com/javase/6/docs/technotes/guides/intl/encoding.doc.html -
> there could be a BOM, that would break the internal logic. So the patch
> enforces UTF_32BE without BOM.

Right, there was a problem with encoding probably.
> 
> Could you please apply the patch on top of "initial patch adding file backed
> pseudoblobs and changes using the new structures". And run the test again?
Now the task passed. I can apply whole patch then.

> If it fails please debug that Testfile with a breakpoint in the Testmethod
> "testSetString_long_String" at the first assert. Then please check your
> temp-directory there you should find two files named "netbeans-db-blob...". It
> would be helpful to have that file.
> 
> Could you also please provide info about your test platform - maybe I can
> create a test installation for me (i run this on ubuntu 11.10 with openjdk).
I'm on Ubuntu 11.10. Details from messages.log
  Product Version         = NetBeans IDE 7.1 (Build 201112071828) (#e649e0c4c10c)
  Operating System        = Linux version 3.0.0-15-generic running on amd64
  Java; VM; Vendor        = 1.6.0_20; Java HotSpot(TM) 64-Bit Server VM 16.3-b01; Sun Microsystems Inc.
  Runtime                 = Java(TM) SE Runtime Environment 1.6.0_20-b02
  Java Home               = /usr/local/share/java/jdk1.6.0_20/jre
  System Locale; Encoding = en (nb); UTF-8

> 
> Thanks Matthias
Comment 16 Jiri Rechtacek 2012-02-14 21:09:49 UTC
The patch was applied in core-main/rev/a9544605b68e. Thanks
Comment 17 Jiri Kovalsky 2012-02-15 14:28:44 UTC
This is a community contribution -> adding NETFIX keyword
Comment 18 matthias42 2012-02-18 21:26:10 UTC
Created attachment 115911 [details]
fix null pointer exceptions at various places

Hey Jiri, when working with main-golden I noticed, that there are some places I overlooked when I did the tests for the supplied patches. I found three places where null pointer exceptions could happen, that need to be caught (please see the diff). I reopened this bug, as this is the place where all the patches are joined in one place.
Comment 19 matthias42 2012-03-27 19:13:02 UTC
*** Bug 210156 has been marked as a duplicate of this bug. ***
Comment 20 matthias42 2012-03-27 19:23:22 UTC
The patch of 2012-02-18 is not complete to cases need to be guarded:

1. The method java.sql.Clob#free or java.sql.Blob#free could be missing. They were added to the _interface_ in jdk6, so drivers compiled against jdk5 will fail. See #209834 for more information and a potential fix (catch java.lang.AbstractMethodError)

2. java.sql.Clob#free or java.sql.Blob#free could throw an SQLException or an SQLFeatureNotSupportedException. I propose to catch SQLException (is superclass of SQLFeatureNotSupportedException) and ignore it. The free is an indication to the database to release resources. I have to assume that the database either does not need this call if it is not supported or the the resources are implicitly cleared, when the resultset is closed.

blob.free()

should be changed to:

try {
   blob.free();
} catch (java.lang.AbstractMethodError err) {
   // Blob gained a new method in jdbc4 (drivers compiled
   // against older jdks don't provide this methid
} catch (SQLException ex) {
   // DBMS failed to free resource or does not support call
   // ignore this, as we can't do more
}

clob.free() should be handled equivalent.
Comment 21 matthias42 2012-03-30 20:46:51 UTC
*** Bug 209834 has been marked as a duplicate of this bug. ***
Comment 22 Jiri Rechtacek 2012-04-02 18:00:34 UTC
Your hints was applied in core-main/rev/3311e4df9e11
Comment 23 schkovich 2012-04-03 11:53:18 UTC
Created attachment 117731 [details]
stacktrace

error in sql query
Comment 24 Jiri Rechtacek 2012-04-03 12:30:45 UTC
core-main/rev/3311e4df9e11
Comment 25 matthias42 2012-04-03 20:41:56 UTC
Created attachment 117759 [details]
fix null pointer exceptions at various places
Comment 26 matthias42 2012-04-03 20:47:58 UTC
(In reply to comment #18)

Sorry! The NULL-pointer exception fixes mentioned in comment #18 are still missing. I attached a rebased version of "fix null pointer exceptions at various places". This was done against core-main as of about 2h ago.

Changes:

DBReadWriteHelper.java:
- Before calling Lob#free check for non-null result (both CLOB and BLOB)

(C/B)lobFieldTableCellEditor:
- make save action sensitive to null values and disable menu entry if null is found
- if saveLobToFile is invoked with a null value silently return
  (this is the real guard, first part is primary for user experience)
Comment 27 Quality Engineering 2012-04-04 10:10:52 UTC
Integrated into 'main-golden', will be available in build *201204040400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/3311e4df9e11
User: Jiri Rechtacek <jrechtacek@netbeans.org>
Log: #206233: Improve BLOB/CLOB Handling;
supplement to matthias42's patch
Comment 28 Jiri Rechtacek 2012-04-05 11:44:18 UTC
(In reply to comment #26)
> (In reply to comment #18)
> 
> Sorry! The NULL-pointer exception fixes mentioned in comment #18 are still
> missing. I attached a rebased version of "fix null pointer exceptions at
> various places". This was done against core-main as of about 2h ago.

More non-null check was applied - core-main/rev/a09f2360e7d8
Thanks.

> 
> Changes:
> 
> DBReadWriteHelper.java:
> - Before calling Lob#free check for non-null result (both CLOB and BLOB)
> 
> (C/B)lobFieldTableCellEditor:
> - make save action sensitive to null values and disable menu entry if null is
> found
> - if saveLobToFile is invoked with a null value silently return
>   (this is the real guard, first part is primary for user experience)