Enable Clob locator support between NetworkServer and NetworkClient and modify tests that experience changed behaviour due to this(enabling Clob Locators).

• Key: DERBY-2702
• Type: Sub-task
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: V.Narayanan
• Reporter: V.Narayanan
• Component/s: Network Client
• Fix Version/s: 10.3.1.4

Description

This issue will address enabling locator support for Clobs between the Network Server and the Network Client. In addition to this, the tests that will experience changed behaviour as a result of this will also be fixed here.

Comments

[V.Narayanan]

This issue will address the changes to enable the use of locators for Clob. This will be divided into the following sub-tasks.

   1) Enable the use of locators.
   2) Modification of tests that experience changed behavior with locators

[V.Narayanan]

This issue will depend on the modification made to

java/client/org/apache/derby/client/am/CallableLocatorProcedures.java

Upon an error, for all procedure calls that specify an locator, check whether
the error was caused by an invalid locator. If yes, report
BLOB_ACCESSED_AFTER_COMMIT, instead.

[V.Narayanan]

The tests depend on the v5 of the patch attached to DERBY-2604
which addresses a few bugs that were found to be existing in the
current Clob locator implementation.

[V.Narayanan]

I made a mistake in the previous linking. It should have said this issue is
*Blocked By* instead of *Blocks*. Sorry for the mistake.

[V.Narayanan]

The BlobClob4BlobTest.testPosition testPositionClobWithUnicode and
testPositionString with unicode are tests that test the position method
for the multibyte unicode characters. These tests were failing as a result
of this bug.

[V.Narayanan]

The tests depend on the v5 of the patch attached to DERBY-2604
which addresses a few bugs that were found to be existing in the
current Clob locator implementation.

[Daniel John Debrunner]

I'm a little confused by this in the first comment:

  
   1) Enable the use of locators.

How does an application request use of locators?

[V.Narayanan]

The createClob() method implementation has been done in this issue.

[V.Narayanan]

My apologies for the ambiguity in the comment. Locators in an internal concept.

An application cannot request for its usage. The NetworkClient will request the NetworkServer to send it a locator
instead of the LOB value.

Enabling was enabling locator support between the Network Client and the NetworkServer

What I meant by this comment was internally locator support has been disabled in the following places and they should
be enabled.

1) createClob() - Does not return a locator enabled Clob as of now. Please note the
                              disabling done at line no 2216 of am/Connection.java for Blob
                              and a similar disabling done in the patch attached that implements
                               createClob() in the patch attached to issue 2587

2) NetResultSetRequest - line no 275. This the place where we (The Network Client) request for
                                                 locators (To the Network Server) if the ResultSet contains a LOB

3) NetStatementRequest - line no 641 where the LID constants are initialized in the SQLDTA that
                                                 is sent.
                              

[V.Narayanan]

By internal concept I mean something that is not known to the user.

[V.Narayanan]

The changes done here are very similar to the test changes done as part
of Derby-2496 to enable Blob locator support. The only difference being
the Clob related tests are addressed here.

M java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/ClobTest.java

- Adjusted tests to not expect Clob objects to be valid after
  transaction commit.

M java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/PreparedStatementTest.java

- Adjusted tests to not expect Clob objects to be valid after
  transaction commit.
- Rewrote testSetClobLengthless to take advantage of the existence of
  Connection.createClob

M java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/ResultSetTest.java

- Adjusted tests to not expect Clob objects to be valid after
  transaction commit.

M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/BlobClob4BlobTest.java

- testClobAfterCommit is changed to test that Blobs on the client is no longer valid after
  transaction has committed.

- testClobAfterClosingConnection has been changed to not expect a Clob to be valid in the
  client after connection has been closed.

- testSelfDestructiveClob2 has been changed to expect an IOException from the Clob
  after the table associated with the Clob is deleted. The IOException that is obtained
  contains a null value when getCause() is used. There seems to be an issue with the
  exception not being changed inside this IOException. The test has been changed to verify
  that an exception is thrown in the NetworkClient case.

M java/client/org/apache/derby/client/net/NetStatementRequest.java

- Enabled the LID constants that would make the NetworkClient request the NetworkServer request
  for locators.

I have done one integrated run of all the patches attached to issues indicated here as dependencies and the test fixes here are directly related to the failures that I encountered.

I am running junit All again after fixing these failures.

I am running junit all with all the dependent patches and shall revert back with the results.

Pls consider this patch for reviews and comments.

[Daniel John Debrunner]

> What I meant by this comment was internally locator support has been disabled in the following places and they should
be enabled.

It's just that I would expect such changes to be performed under this issue, since it's summary implies it's a change for testing.

[V.Narayanan]

you are correct. I am sorry. I should have either said in the summary of
the issue that

I am Enabling locator support between client and server and fixing tests
that experience test failures as a result of that
or
Opened seperate issues for enabling locators.

This was stupid of me.

I do not know if I am allowed to change a JIRA issue subject. I will do
this if this is possible and if
people think this is ok.

Otherwise I will raise seperate JIRA issues.

[V.Narayanan]

updated the issue to reflect the fact that it handles both the enabling of locator support and the tests that experience changed behaviour as a result of this. If this change is not satisfactory and people still feel that it is confusing to address both there issues here, I will change this into two seperate issues.

[V.Narayanan]

I integrated this patch with all the dependent patches and ran junit All. There were no failures in the
test run.

The following is a small description of the step-by-step procedure adopted during the
integration process. I applied the below on a fresh workspace.

1) DERBY-2496 Enabling Blob support for locators is committed.
   So I did not have to apply any patch for this dependency.
   

2) DERBY-2694 org.apache.derby.impl.drda.DDMWriter uses wrong algorithm to avoid
                             spliting varchar in the middle of a multibyte char.
   Applied the patch attached to this issue (derby-2694.diff)

3) DERBY-2587 Connection.createClob() and Connection.createBlob() need to return
                             locator support enabled LOB objects in the NetworkClient

   Part of the patch related to modifying createBlob() to return locator support
   enabled Blobs is already committed.

   I applied ConnectionLocatorWork_v4.diff to the workspace.

   In addition to applying this patch I had to do one more thing. The createClob()
   method modified in this patch has been temporarily disabled from sending
   locator enabled Clobs.I had to remove the (&& false) intentional disabling
   that was done.

   I am hoping that I can submit a intermediate patch to remove (&& false)
   between the commit of v4 attached to this issue and 2702 being committed.

   
4) DERBY-2604 Implement Clob support for Locators

   A patch was attached to this issue addressing some bugs found in the
   implementation.

   Applied ClobLocatorWork_v5.diff to the workspace.

5) DERBY-2702 Enable Clob locator support between NetworkServer and NetworkClient and modify tests that
                             experience changed behaviour due to this(enabling Clob Locators).

   Applied ClobLocatorWorkTestModifications.diff

6) I then did a

   ant Clobber && ant all && ant buildjarsclean

   on the above workspace.

7) The jars obtained from the above build was then used to run junit All.


I had a clean run. No failures were observed.

Pls find the integrated patches attached here for further reference.

Pls note that the integrated patches are *not for commit*.

[V.Narayanan]

Pls note that these patches are *not for a commit*.

[V.Narayanan]

I ran tests(junit All) on the integrated patch again yesterday evening. I did not see any failures in the runs.

I ran tests on the solaris platform.

[V.Narayanan]

In the step 3) of the series of steps I had outlined to run the integrated
patch I had mentioned the following

  "In addition to applying this patch I had to do one more thing. The createClob()
   method modified in this patch has been temporarily disabled from sending
   locator enabled Clobs.I had to remove the (&& false) intentional disabling
   that was done."

The createClob() related locator work has been committed.

I will be submitting a follow-up patch to this issue which will have
createClob() locator related functionality enabled.

Pls note that this had been done while running the integrated tests.
This change can be seen in ClobLocatorWorkTestFailuresFixes_NOT_FOR_COMMIT.diff

[V.Narayanan]

As indicated in the comments above the createClob method
related to locators has been enabled in this patch. I have not
yet run tests on this patch.

I will run the tests and shall post the results.

Please consider this patch for reviews and comments.

[Knut Anders Hatlen]

Is the use of the constants DRDA_TYPE_NLOBLOC and DRDA_TYPE_LOBLOC for clobs in NetStatementRequest correct? I would have thought that DRDA_TYPE_NCLOBLOC and DRDA_TYPE_CLOBLOC should have been used instead.

[V.Narayanan]

You are correct Knut, I will correct this and revert back with the test
results again. Thank you for pointing this out.

[V.Narayanan]

I have corrected the issue pointed out with the LID
constants

DRDAConstants.DRDA_TYPE_NLOBLOC has been
replaced with DRDAConstants.DRDA_TYPE_NCLOBLOC

 DRDAConstants.DRDA_TYPE_LOBLOC has been
replaced with DRDAConstants.DRDA_TYPE_CLOBLOC

I have not yet run tests on this patch. I shall run the tests and
shall revert back with the test results.

Pls consider v3 for reviews and comments.

[V.Narayanan]

I ran tests on v3 and observered no failures. If everything is OK I request for v3 to be
considered for a commit.

[Knut Anders Hatlen]

Committed revision 544481.

[V.Narayanan]

Thanks a ton for this commit Knut.

With this locator support on the Clob is also up and running :-).

Thanks again for the guidance and the reviews offered throughout the process
of development.

[V.Narayanan]

All patches submitted and committed.