[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-05-01 Thread Thomas Groh (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990999#comment-15990999
 ] 

Thomas Groh commented on BEAM-2123:
---

e.g. {{NullableCoder.of(StringUtf8Coder.of());}} can return the original String 
when non-null, and an object that is equal only to other encoded null-string 
bytes when null.

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
> Fix For: Not applicable
>
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-05-01 Thread Thomas Groh (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990998#comment-15990998
 ] 

Thomas Groh commented on BEAM-2123:
---

{{null}} is a valid value if the coder supports encoding null values. If not, 
the coder will or should throw an appropriate exception, but we cannot know if 
the coder does or does not support null values from within the 
{{StructuredCoder}} base class.

For ease of use elsewhere in the SDK, {{structuralValue}} does not throw 
{{IOExceptions}}, as it should always be possible for a Coder to encode to an 
in-memory byte array given valid input.

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
> Fix For: Not applicable
>
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-04-30 Thread Ted Yu (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990529#comment-15990529
 ] 

Ted Yu commented on BEAM-2123:
--

Thanks for the confirmation (I didn't see structuralValue() either)

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-04-30 Thread Daniel Halperin (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990527#comment-15990527
 ] 

Daniel Halperin commented on BEAM-2123:
---

https://github.com/apache/beam/blob/a198f8d232bda1ae68467b17027565d9fcef63a8/sdks/java/core/src/test/java/org/apache/beam/sdk/coders/StructuredCoderTest.java#L45

I typoed -- NullBooleanCoder not Nullable, but it should have been obvious what 
I meant.

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-04-30 Thread Ted Yu (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990526#comment-15990526
 ] 

Ted Yu commented on BEAM-2123:
--

I looked at 
./sdks/java/core/src/test/java/org/apache/beam/sdk/coders/StructuredCoderTest.java
 but didn't see NullableBooleanCoder

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-04-30 Thread Daniel Halperin (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990486#comment-15990486
 ] 

Daniel Halperin commented on BEAM-2123:
---

There is one in {{StructuredCoderTest}}: {{NullableBooleanCoder}}. I suspect 
{{NullableCoder}} could be turned into one of these, too.

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-04-30 Thread Ted Yu (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990387#comment-15990387
 ] 

Ted Yu commented on BEAM-2123:
--

I am interested in seeing a counter example where null value produces 
meaningful output.

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (BEAM-2123) Passing potential null pointer to encode() in StructuredCoder#structuralValue

2017-04-30 Thread Daniel Halperin (JIRA)

[ 
https://issues.apache.org/jira/browse/BEAM-2123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15990311#comment-15990311
 ] 

Daniel Halperin commented on BEAM-2123:
---

It looks like you believe `null` is not a valid value for any structured coder. 
Is that actually true? I don't believe so.

It's up to the extending coder to throw on `null` values.

> Passing potential null pointer to encode() in StructuredCoder#structuralValue
> -
>
> Key: BEAM-2123
> URL: https://issues.apache.org/jira/browse/BEAM-2123
> Project: Beam
>  Issue Type: Bug
>  Components: sdk-java-core
>Reporter: Ted Yu
>Assignee: Thomas Groh
>Priority: Minor
>
> {code}
>   public Object structuralValue(T value) {
> if (value != null && consistentWithEquals()) {
>   return value;
> } else {
>   try {
> ByteArrayOutputStream os = new ByteArrayOutputStream();
> encode(value, os, Context.OUTER);
> {code}
> If value is null, encode() would throw CoderException (I checked 
> ByteArrayCoder and KvCoder) which would be caught and converted to 
> IllegalArgumentException.
> Looks like structuralValue() can check null value directly and throw 
> CoderException. This would result in clearer exception.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)