[jira] Commented: (DERBY-2346) Provide set methods for clob for embedded driver

["Knut Anders Hatlen (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DERBY-2346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12488070 ] 

Knut Anders Hatlen commented on DERBY-2346:
-------------------------------------------

Could you elaborate on how String.getBytes() is unreliable? As far as
I can tell, ClobStreamControl.getByteFromString() does exactly the
same as string.getBytes("UTF-8").

I haven't done a thorough review, but here are my comments after a
quick look at the patch:

* It seems a bit odd to me that a number of the methods in
  ClobStreamControl and LOBStreamControl throw both IOException and
  SQLException. I would have expected that StandardException was used
  instead of SQLException at this level of the code. Throwing
  SQLException at this level tends to lead to awkward code other
  places. For instance, EmbedClob's constructor must catch the
  SQLException and convert it to a StandardException because of this
  (and the StandardException will be converted back to an SQLException
  later).

* Typo in LOBInputStreeam.isObsoloate: should have been isObsolete?

* Typo in ClobStreamControl.getStreamPostition: extra t in position

* ClobStreamControl.getCharLength: long ret = reader.read(dummy) could
  use int instead of long since Reader.read(char[]) returns an int.

* Wouldn't the code be simpler (and possibly more efficient) if the
  switch statement in ClobStreamControl.getStreamPosition were
  replaced with if/else if? For instance, this part of the switch
+            switch (c >> 4) {
+                case 0: case 1: case 2: case 3: case 4: case 5: case 6: case 7:
  is equivalent to this simple if statement:
             if ((c & 0x80) == 0)

* Would it be better to throw an exception instead of returning -1
  when getStreamPosition() reads past the end of the stream? I think
  reading past the end of the stream is always an error, so if an
  exception is thrown, the callers don't have to check for a negative
  return value.

* ClobStreamControl.insertString doesn't look correct to me. Its
  javadoc comment says that it returns the current byte position, but
  it always returns str.length(). It also seems like byte positions
  and char positions are mixed in this line:
+        endPos = (endPos < 0) ? getCharLength() : pos + endPos;
  endPos and pos are byte positions, whereas getCharLength() returns
  the length in characters.

* In LOBStreamControl.replaceBytes, this loop might never terminate if
  end-of-file is reached:
+            while (sz != 0) {
+                int readLen = (int) Math.min (1024, sz);                
+                int actualLength = oldFile.read (tmpByte, 0, readLen);
+                tmpFile.write (tmpByte, 0, actualLength);
+                sz -= actualLength;
+            }

And won't this loop end up passing rdLen == -1 to write()?
+            if (endPos < length) {
+                do {
+                    rdLen = oldFile.read (tmpByte, 0, 1024);
+                    tmpFile.write (tmpByte, 0, rdLen);
+                }while (rdLen >= 1024);
+            }            

> Provide set methods for clob for embedded driver
> ------------------------------------------------
>
>                 Key: DERBY-2346
>                 URL: https://issues.apache.org/jira/browse/DERBY-2346
>             Project: Derby
>          Issue Type: Sub-task
>          Components: JDBC
>    Affects Versions: 10.3.0.0
>            Reporter: Anurag Shekhar
>         Assigned To: Anurag Shekhar
>         Attachments: derby-2346-only_for_review.diff, derby-2346.v1.diff
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.