You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Kristian Waagan (JIRA)" <ji...@apache.org> on 2008/07/09 15:53:32 UTC
[jira] Created: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Create a utility class for skipping data in an InputStream
----------------------------------------------------------
Key: DERBY-3770
URL: https://issues.apache.org/jira/browse/DERBY-3770
Project: Derby
Issue Type: Improvement
Components: Miscellaneous
Affects Versions: 10.5.0.0
Reporter: Kristian Waagan
Priority: Minor
The contract of InputStream.skip is somewhat difficult, some would even say broken.
See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
Suggested functionality:
- long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
- void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
I know of two different approaches, both skipping in a loop:
a) Verify EOF with a read call when skip returns zero.
b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12637467#action_12637467 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Before we close this issue, we should make sure the new utility methods are used these places:
EmbedBlob.length()
PositionedStoreStream.reposition() (and remove skipFully() from that class)
UpdatableBlobStream.updateIfRequired()
SQLBinary.getLength()
The next methods look buggy. They assume that InputStream.skip() or DataInput.skipBytes() always skip the requested number of bytes, unless EOF is reached. They should use skipFully() instead.
CompressedNumber.skipInt(DataInput)
CompressedNumber.skipInt(InputStream)
CompressedNumber.skipLong(DataInput)
CompressedNumber.skipLong(InputStream)
StoredFieldHeader.readFieldDataLength()
StoredPage.readRecordFromStream()
StoredPage.skipField()
Same issue exists in the next two methods, but in their case the stream variable is declared as an ArrayInputStream, which is guaranteed to skip as much as requested, as long as EOF is not reached. We should either use skipFully() or comment why we expect skipBytes() to do the right thing.
StoredPage.readOneColumnFromPage()
StoredPage.readRecordFromArray()
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Junjie Peng updated DERBY-3770:
-------------------------------
Attachment: derby-3770-2.patch
derby-3770-2.stat
Thanks for your attention, Mamta. I have receive your comments just now. Sorry to reply late.
<<Junjie, the patch is commented pretty well and the code changes for those comments look good.
<<One comment for the engine code change
<<1)The 2 new methods skipFully(InputStream is) and skipFully(InputStream is, long skippedBytes) in their javadocs only talk about IOException and EOFException for skipFully(InputStream is, long skippedBytes). Should we put NullPointerException() also in the javadoc?
-----I have add the declaration for NullException.
<<Just couple comments for the new junit test
<<1)testNullStream has 2 test cases to check for null inputstream. For some reason, if no NullPointerException is thrown, then we have following to catch it
<<fail("Null InputStream is refused!");
<<The error message looks misleading. Should it be saying something like
<<fail("Null InputStream is accepted!");
-----I have correct it.
<<2)The 2 tests in testNullStream only check for NullPointerException. Shouldn't we be catching other exceptions and make the test fail for those exceptions.
-----I'm not clear about this. What other exceptions should be tested int testNullStream()? For EOFException, I have tested it in testSkipFully(). As to IOException, excluding EOFException, I don't know how to create or simulate it. Could you give me more advices?
<<3)Don't have to address this but should we consider combining testSkipUtilEOFWithOddLength and testSkipUtilEOF into one test fixutre.
-----testSkipUtilEOFWithOddLength() only tests EOF with special length, I think it's better to seperate it from common length. Is the name of the method not clear? Is testSkipUtilEOFWithSpecialLength() better?
<<Thanks for working on this jira entry.
Mamta, please give more suggestion to improve the patch. Thanks again!
Regards
Junjie
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624649#action_12624649 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Committed revision 688049.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12644602#action_12644602 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
The regression tests passed. Committed revision 709900.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat, derby-3770-remove-1a.diff, derby-3770-use-1a.diff
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen updated DERBY-3770:
--------------------------------------
Attachment: derby-3770-use-1a.diff
Attaching a patch to make the utility methods used more places in the code (derby-3770-use-1a.diff). It replaces all the skip loops with calls to skipFully() or skipUntilEOF(). With this patch, the first four methods in the list above use the utility methods.
All the regression tests ran cleanly.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat, derby-3770-use-1a.diff
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624980#action_12624980 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Good points, Dan.
As to the purpose of the method that skips until EOF, that's the approach we use to find the length of a resettable stream: move to EOF, count the bytes on the way, and reset the stream. Probably clearer to name the method skipUntilEOF instead of skipFully, though.
I didn't notice before now, but the class iapi.io.InputStreamUtil contains a method skipBytes(InputStream,long) that looks identical to iapi.util.StreamUtil.skipFully(InputStream,long). Probably better to add more methods to that class, I agree.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624304#action_12624304 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
The handling of unexpected exceptions looks fine to me. Since they are not caught explicitly, they will propagate out to the JUnit framework and be reported correctly there.
It may be slightly clearer, though, if we replace assertTrue(true) with just a comment like this:
catch (NullPointerException npe) {
// ignoring expected exception
}
The StreamUtil class imports sun.tools.tree.NullExpression, which seems wrong. Also, the javadoc comments in that class say "@throws NullExpression", whereas they should have said "@throws NullPointerException".
It's probably also a good idea to move the code from UTF8Util.skipPersistent() into the StreamUtil class, since that method doesn't have anything to do with UTF-8 and therefore making it non-private in the UTF8Util class may cause some confusion.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12627693#action_12627693 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Thanks!
Committed revision 691253.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624633#action_12624633 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Thanks, Junjie!
The patch looks good to me. I'll run some tests and commit the patch if there are no problems.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan closed DERBY-3770.
----------------------------------
Utility class finished.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Fix For: 10.5.0.0
>
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat, derby-3770-remove-1a.diff, derby-3770-use-1a.diff
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Junjie Peng updated DERBY-3770:
-------------------------------
Attachment: derby-3770-4.stat
derby-3770-4.patch
Hi, Mamta and Daniel. Thanks for your advices. I have done some improvement.
1.) Delete StreamUtil, move the methods to InputStreamUtil, and move the test class to suitable place.
2.) Rename SKIP_BUFFER_SIZE to SKIP_FRAGMENT_SIZE to keep clear.
3.) Use " if (skippedNow == 0)" in skipPersistent().
4.) About skipByte(InputStream,long):
/**
Skip a number of bytes in the stream. Note that this version takes and returns
a long instead of the int used by skipBytes.
@exception IOException if an I/O error occurs.
@exception EOFException if the end of the stream is reached
@see DataInput#skipBytes
*/
public static long skipBytes(InputStream in, long n) throws IOException {
while (n > 0) {
//System.out.println(" skip n = " + n);
long delta = in.skip(n);
//System.out.println(" skipped = " + delta);
if (delta < 0)
throw new EOFException();
n -= delta;
}
return n;
}
This method doesn't work well. First, for "long delta = in.skip(n); ", delat won't to be negative, so we can not judge EOFException with "if (delta < 0)". The method skipPersistent() is fittest to judge EOF has arrived.
So, I deleted skipBytes(), and replace it with skipFully() where skipBytes() is used.
5.) Daniel said "skipPersistent() states that if a fewer number of bytes is skipped then it is guaranteed that eof has been reached, but skipFully() does not take advantage of this, instead it will always perform an extra call to skipPersistent(). " Howeve, skipPersistent() is useful to skipFully(), it can guarante that requested num of bytes will be skipped most probably. If we use the common skip() method, we can not judge enough bytes has been skipped fully even having not EOFEception.
Please check the new patch, thanks!
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12627475#action_12627475 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Thank you!
The patch looks good to me. I have started the regression tests and plan to commit it if there aren't any failures.
Some small things that we may consider to change after the commit:
1) Should the test be placed under unitTests/junit instead of functionTests/tests/engine? The existing tests under functionTests/tests/engine seems to boot the full Derby engine, whereas the test in the patch only tests a single internal class and probably fits better under unitTests/junit.
2) In skipUntilEOF, the scope of the local variable r could be narrowed down (it could be declared in the body of the while loop).
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Junjie Peng updated DERBY-3770:
-------------------------------
Attachment: derby-3770-6.stat
derby-3770-6.patch
OK, Knut. I have adopted your advice. Please check it!
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Mamta A. Satoor (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622807#action_12622807 ]
Mamta A. Satoor commented on DERBY-3770:
----------------------------------------
Junjie, I will look at the patch soon but if you get a chance, can you put a brief description of the logic of the patch in this jira entry?
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Junjie Peng updated DERBY-3770:
-------------------------------
Attachment: derby-3770-5.stat
derby-3770-5.patch
HI, Knut. I adopted your advice, please check the patch. Thanks!
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624285#action_12624285 ]
Junjie Peng commented on DERBY-3770:
------------------------------------
Mamta, thanks for your adivice.
I have contemplated your comment , I think the test is OK in this situation. The NPE is checked first when calling the skipFully() method, so no other kind of exception will be thrown. What's your opinion?
As to the "more graceful way of catching unexpected exceptions", above all, thanks for your advice, it helps me understand the test framework better. However, I haven't found known tools to realize it, so I would leave it as it's now.
Regards
Junjie
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12626097#action_12626097 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Thanks for the new patch. It basically looks good. A couple of small issues:
1) Package and class name in the header of InputStreamUtilTest should be updated.
2) I think Dan's point with skipFully (now skipUntilEOF) was that you don't necessarily have to call skipPersistent until it returns 0. It is OK to stop calling it once it returns less bytes than requested. So to reduce the number of times skipPersistent is called, skipUntilEOF could do something like this:
long bytes = 0;
while (true) {
long skipped = skipPersistent(is, SKIP_FRAGMENT_SIZE);
bytes += skipped;
if (skipped < SKIP_FRAGMENT_SIZE) {
return bytes;
}
}
3) I noticed that SKIP_FRAGMENT_SIZE had been lowered from 1024*1024 to 512*1024 in this patch. I don't think there's any reason to keep this constant small. There shouldn't be any disadvantages with having a higher value, so it might be better to set it to a very high value, for instance Integer.MAX_VALUE.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Mamta A. Satoor (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622815#action_12622815 ]
Mamta A. Satoor commented on DERBY-3770:
----------------------------------------
Junjie, the patch is commented pretty well and the code changes for those comments look good.
One comment for the engine code change
1)The 2 new methods skipFully(InputStream is) and skipFully(InputStream is, long skippedBytes) in their javadocs only talk about IOException and EOFException for skipFully(InputStream is, long skippedBytes). Should we put NullPointerException() also in the javadoc?
Just couple comments for the new junit test
1)testNullStream has 2 test cases to check for null inputstream. For some reason, if no NullPointerException is thrown, then we have following to catch it
fail("Null InputStream is refused!");
The error message looks misleading. Should it be saying something like
fail("Null InputStream is accepted!");
2)The 2 tests in testNullStream only check for NullPointerException. Shouldn't we be catching other exceptions and make the test fail for those exceptions.
3)Don't have to address this but should we consider combining testSkipUtilEOFWithOddLength and testSkipUtilEOF into one test fixutre.
Thanks for working on this jira entry.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Daniel John Debrunner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624987#action_12624987 ]
Daniel John Debrunner commented on DERBY-3770:
----------------------------------------------
Minor point on the skipPersistent method, it has the following code:
long skippedNow = in.skip(bytesToSkip - skipped);
if (skippedNow <= 0)
but skippedNow can never be negative so to be clearer the code should be
long skippedNow = in.skip(bytesToSkip - skipped);
if (skippedNow == 0)
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Junjie Peng updated DERBY-3770:
-------------------------------
Attachment: derby-3770-3.stat
derby-3770-3.patch
Hi, Knut. Thanks for your advice.
1.)---test framework. I agree with your method to add comment " // ignoring expected exception ". However, as what I used is just like Andrew suggested in his <Pragmatic Unit Testing>, I think it can work well.
2.)---wrong import. I have corrected in the new patch.
3.)---move the code from UTF8Util.skipPersistent() into the StreamUtil class. It's a good suggestion, I have adopted it.
Please check the patch!
Regards
Junjie
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen updated DERBY-3770:
--------------------------------------
Attachment: derby-3770-remove-1a.diff
The possibly buggy skip methods in CompressedNumber aren't used anywhere in the code, except in CompressedNumberTest where the DataInput variants are tested. Instead of fixing methods that aren't used, I think it is better to remove them. The attached patch (derby-3770-remove-1a.diff) removes all the skip methods from CompressedNumber and the test cases for these methods. It also removes another unused method, readIntAndReturnIntPlusOverhead().
I have started the regression tests on the patch.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat, derby-3770-remove-1a.diff, derby-3770-use-1a.diff
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Junjie Peng reassigned DERBY-3770:
----------------------------------
Assignee: Junjie Peng
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Junjie Peng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Junjie Peng updated DERBY-3770:
-------------------------------
Attachment: derby-3770-1.stat
derby-3770-1.patch
Hi, Kristian. Please check the patch, thanks!
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen resolved DERBY-3770.
---------------------------------------
Resolution: Fixed
Fix Version/s: 10.5.0.0
The rest of the methods are working on DataInputs, not InputStreams, so the utility methods cannot be used there. I've logged DERBY-3941 to handle the remaining problems. Marking this issue as resolved.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Fix For: 10.5.0.0
>
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat, derby-3770-remove-1a.diff, derby-3770-use-1a.diff
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12644579#action_12644579 ]
Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------
Committed revision 709856.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat, derby-3770-5.patch, derby-3770-5.stat, derby-3770-6.patch, derby-3770-6.stat, derby-3770-use-1a.diff
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Daniel John Debrunner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624901#action_12624901 ]
Daniel John Debrunner commented on DERBY-3770:
----------------------------------------------
Some questions/comments about skipFully(InputStream is)
What is the purpose of this method, when would it be used? Skipping until EOF seems a useless operation.
SKIP_BUFFER_SIZE is a somewhat confusing name since no buffer is ever allocated.
skipPersistent() states that if a fewer number of bytes is skipped then it is guaranteed that eof has been reached, but skipFully() does not take advantage of this, instead it will always perform an extra call to skipPersistent().
Other input stream utility methods are in org.apache.derby.iapi.services.io, any reason to have this new class in a different package?
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3770) Create a utility class for skipping
data in an InputStream
Posted by "Mamta A. Satoor (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624067#action_12624067 ]
Mamta A. Satoor commented on DERBY-3770:
----------------------------------------
Junjie, sorry for not getting back to you sooner.
What I meant bu comment 2) for the tests is something along following line. In most of the JDBC junit tests in Derby, if say executing a specific query is only allowed to send a specific exception, then we assert that using following (s below is java.sql.Statement)
assertStatementError("42Y55", s, "CALL SYSCS_UTIL.SYSCS_UPDATE_STATISTICS('APP','T1',null)");
So, if the query above throws any exception other than "42Y55" then that will cause the junit test to fail saying that it expected 42Y55 but it got something else.
I was wonderinf in the test in question here, if there was anyway of catching exceptions other than NPE
+ try{
+ StreamUtil.skipFully(null);
+ fail("Null InputStream is accepted!");
+ }catch (NullPointerException e) {
+ assertTrue(true);
+ }
I guess, if the test case above did get an exception other than NPE, we will just get out of the test fixture with that exception. I was curious if there was some more graceful way of catching unexpected exceptions like we do for jave.sql.Statement with assertStatementError. This is not a biggie and feel free to not address this issue if there is no simple way of doing what assertStatementError does.
> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
> Key: DERBY-3770
> URL: https://issues.apache.org/jira/browse/DERBY-3770
> Project: Derby
> Issue Type: Improvement
> Components: Miscellaneous
> Affects Versions: 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Junjie Peng
> Priority: Minor
> Attachments: derby-3770-1.patch, derby-3770-1.stat, derby-3770-2.patch, derby-3770-2.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip procedure throughout the Derby code base.
> Suggested functionality:
> - long skipFully(InputStream) : skips until EOF, returns number of bytes skipped
> - void skipFully(InputStream,long) : skips requested number of bytes, throws EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
> a) Verify EOF with a read call when skip returns zero.
> b) Throw EOFException if skip returns zero before requested number of bytes have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, could be put in the same package?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.