You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "James Baldassari (Created) (JIRA)" <ji...@apache.org> on 2012/01/25 10:16:40 UTC
[jira] [Created] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Insufficient validation in generated specific record builder implementations
----------------------------------------------------------------------------
Key: AVRO-1007
URL: https://issues.apache.org/jira/browse/AVRO-1007
Project: Avro
Issue Type: Bug
Affects Versions: 1.6.1
Reporter: James Baldassari
Assignee: James Baldassari
The are two main problems with the generated build() method in specific record builders:
* For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
* For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Doug Cutting (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203928#comment-13203928 ]
Doug Cutting commented on AVRO-1007:
------------------------------------
According to the spec, that schema is malformed. The default value of a union should always be interpreted as the first type in the union.
I don't think this schema currently works when reading records that lack the field "f". See line 348 of ResovlingGrammarGenerator, where the default value is encoded using the first type in the union.
I suppose we could change the spec to permit a null default value if any element of the union is null, but I don't see why we should. It makes the spec more complex and doesn't provide any additional expressive power.
This patch would make the builder API enforce the spec, consistent with ResolvingDecoder.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204120#comment-13204120 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
This approach works for me and the code is clear.
Minor nit we should fix at the same time: RecordBuilderBase.validate() has incorrect javadoc, it says it throws NullPointerException but it throws AvroRuntimeException.
However, there is a new test failure in TestSpecificRecordBuilder.testInterop() now that we are more strict and a null field without a default must be set.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203883#comment-13203883 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
Should this work if any of the union branches are null, not just the first branch? Although the spec mentions that a union default applies to the first branch, there seems to be an implicit exception for null in the Java implementation. The following has always worked:
{"type":"record", "name":"Rec", "fields":[{"name":"f", "type":["string", "null"], "default":null}]}
The BuilderAPI does not need to hold to the spec however, and if any branch is null, an unset field is ok IMO.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Scott Carey updated AVRO-1007:
------------------------------
Fix Version/s: 1.6.2
Status: Patch Available (was: Open)
This has a patch to review. Should this be fixed for 1.6.2, or only 1.7.0 ?
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "James Baldassari (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
James Baldassari updated AVRO-1007:
-----------------------------------
Attachment: AVRO-1007-v3.patch
I just realized that this issue also affects the generic builder. Here's a third iteration of the patch (including new unit tests) that also improves validation in the generic builder
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: [jira] [Commented] (AVRO-1007) Insufficient validation in
generated specific record builder implementations
Posted by Scott Carey <sc...@apache.org>.
Your email address is registered with the avro-dev mailing list.
See:
http://avro.apache.org/mailing_lists.html
You would not be able to send mail to this list if you were not
subscribed.
On 2/8/12 11:40 AM, "info@johnmasters.com" <in...@johnmasters.com> wrote:
>Why am I on this email string? Please remove me.
>
>--- On Wed, 2/8/12, Scott Carey (Commented) (JIRA) <ji...@apache.org>
>wrote:
>
>From: Scott Carey (Commented) (JIRA) <ji...@apache.org>
>Subject: [jira] [Commented] (AVRO-1007) Insufficient validation in
>generated specific record builder implementations
>To: dev@avro.apache.org
>Date: Wednesday, February 8, 2012, 2:38 PM
>
>
> [
>https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.pl
>ugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203889#comm
>ent-13203889 ]
>
>Scott Carey commented on AVRO-1007:
>-----------------------------------
>
>Slight clarification:
>
>The schema above has worked for me in the past, but I am not sure if it
>has _always_ worked -- that was too strong of a statement.
>
>> Insufficient validation in generated specific record builder
>>implementations
>>
>>-------------------------------------------------------------------------
>>---
>>
>> Key: AVRO-1007
>> URL: https://issues.apache.org/jira/browse/AVRO-1007
>> Project: Avro
>> Issue Type: Bug
>> Affects Versions: 1.6.1
>> Reporter: James Baldassari
>> Assignee: James Baldassari
>> Labels: java
>> Fix For: 1.6.2
>>
>> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch,
>>AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>>
>>
>> The are two main problems with the generated build() method in specific
>>record builders:
>> * For non-primitive types, if there is no default value and the user
>>does not set the value, build() will execute successfully without
>>throwing an exception
>> ** Instead, an AvroRuntimeException should be thrown with an exception
>>message indicating the name of the required field that was not set
>> * For primitive types, if there is no default value and the user does
>>not set the value, an AvroRuntimeException is thrown with the 'cause'
>>set to a NullPointerException, which is not very helpful
>> ** The NPE comes from attempting to set the primitive field to the
>>result of defaultValue(), which is null
>
>--
>This message is automatically generated by JIRA.
>If you think it was sent incorrectly, please contact your JIRA
>administrators:
>https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
>For more information on JIRA, see: http://www.atlassian.com/software/jira
>
>
Re: [jira] [Commented] (AVRO-1007) Insufficient validation in generated specific record builder implementations
Posted by in...@johnmasters.com.
Why am I on this email string? Please remove me.
--- On Wed, 2/8/12, Scott Carey (Commented) (JIRA) <ji...@apache.org> wrote:
From: Scott Carey (Commented) (JIRA) <ji...@apache.org>
Subject: [jira] [Commented] (AVRO-1007) Insufficient validation in generated specific record builder implementations
To: dev@avro.apache.org
Date: Wednesday, February 8, 2012, 2:38 PM
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203889#comment-13203889 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
Slight clarification:
The schema above has worked for me in the past, but I am not sure if it has _always_ worked -- that was too strong of a statement.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203889#comment-13203889 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
Slight clarification:
The schema above has worked for me in the past, but I am not sure if it has _always_ worked -- that was too strong of a statement.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "James Baldassari (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
James Baldassari updated AVRO-1007:
-----------------------------------
Attachment: AVRO-1007-v2.patch
Updated the patch to fix unit test failures in TestSpecificErrorBuilder and TestSpecificCompilerTool
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Attachments: AVRO-1007-v2.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "James Baldassari (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13202078#comment-13202078 ]
James Baldassari commented on AVRO-1007:
----------------------------------------
These changes require regenerating/recompiling the specific record classes. However, all of these changes should be backwards-compatible with 1.6. So if this goes out in 1.6.2 and a user doesn't regenerate the specific records, everything should still work.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203869#comment-13203869 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
+1 to Doug's last patch. Tests fail without the one line change in RecordBuilderBase, and succeed with it.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204021#comment-13204021 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
With this patch, the below tests are not consistent. IMO these should all pass, or all fail. No record has a default, so if any of these were missing during schema resolution it would fail. If we want the builder API to behave similar to schema resolution all should fail. If we want an unset field to behave as if it were set to null, then all should pass.
{code}
Schema rec = new Schema.Parser().parse("{\"type\":\"record\", \"name\":\"Nothing\", \"fields\":[" +
"{\"name\":\"blank\", \"type\": \"null\"}]}");
Schema unionSchema = new Schema.Parser().parse("{\"type\":\"record\", \"name\":\"Person\", \"fields\":[" +
"{\"name\":\"address\", \"type\": [\"null\", \"string\"]}]}");
Schema unionSchema2 = new Schema.Parser().parse("{\"type\":\"record\", \"name\":\"Person\", \"fields\":[" +
"{\"name\":\"address\", \"type\": [\"string\",\"null\"]}]}");
@Test
public void buildNoFieldSet() {
new GenericRecordBuilder(rec).build();
}
@Test
public void buildNoFieldSetUnion() {
new GenericRecordBuilder(unionSchema).build();
}
@Test
public void buildNoFieldSetUnion2() {
new GenericRecordBuilder(unionSchema2).build();
}
{code}
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Doug Cutting (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203839#comment-13203839 ]
Doug Cutting commented on AVRO-1007:
------------------------------------
Seems like we're generating a lot of code that looks very similar. Let me see if I can simplify this...
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Doug Cutting (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-1007:
-------------------------------
Attachment: AVRO-1007.patch
Good point. It fixes builder to be consistent with ResolvingDecoder and the spec in using the first type of the union for defaults, but ot does not fix the issue you mention aove, where no default is specified yet a null is the first type in the union.
ResolvingDecoder does currently implement this correctly, failing if no default value is specified in the schema even if the schema is a union with null as its first value.
Here's a patch that fixes the builder API to conform to the spec in this case too.
There's a separate question about whether we should change the spec so that the default value for default values is not unspecified but null. That would permit folks to use schemas like the one you have above. It would require some minor changes to ResolvingDecoder and to this patch.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13202043#comment-13202043 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
This looks good. Do we want to make this sort of change in 1.6.x? Users who have relied upon the builder API to create objects that are invalid and later set them to be valid will break. However, I don't think there are very many users of the builder API yet and we could better javadoc the behavior of build() to indicate the conditions that it will succeed and fail.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Attachments: AVRO-1007-v2.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "James Baldassari (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
James Baldassari updated AVRO-1007:
-----------------------------------
Attachment: AVRO-1007.patch
Here's a patch for record.vm along with some new test cases in TestSpecificRecordBuilder.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Attachments: AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203873#comment-13203873 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
Race condition: My last +1 was to the version prior to the union handling. Looking at the new one now.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Scott Carey updated AVRO-1007:
------------------------------
Attachment: AVRO-1007-v4.patch
This patch (v4) rebases the (v3) version to apply clean to trunk @rev 1242036
It works and passes tests with the change, and tests fail without the change.
+1
Thanks!
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Doug Cutting (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-1007:
-------------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
I fixed RecordBuilderBase and TestSpecificRecordBuilder as suggested and committed this.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Doug Cutting (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-1007:
-------------------------------
Attachment: AVRO-1007.patch
Here's a new version that also handles unions with null and adds a test case for that.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Scott Carey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203991#comment-13203991 ]
Scott Carey commented on AVRO-1007:
-----------------------------------
I am not sure I see how this makes the Builder API respect the spec. Does it allow the following?
{noformat}
{"type":"record", "name":"Person", "fields":[
{"name":"address", "type": ["null", "string"]}
]}
Person.newBuilder().build(); // should error, address has no explicit default
{noformat}
?
If so, to adhere to the spec that should throw an error since address was not set and has no specified default.
Or, if the above is allowed, it is consistent to allow it regardless of the order in the union. Either we enforce that all fields without a specified default must be set, or allow for nullable fields to build without being set. Half way in between seems inconsistent. In short, either we interpret an unset field as an unset field that is only valid if there is a default, or we interpret it as a null, which is valid for null schemas or unions with any branch null.
A related case is below:
{noformat}
{"type":"record", "name":"Nothing", "fields":[
{"name":"blank", "type": "null"}
]}
Nothing.newBuilder().build(); // should error, blank not set and has no explicit default
{noformat}
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (AVRO-1007) Insufficient validation in generated
specific record builder implementations
Posted by "Doug Cutting (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-1007?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-1007:
-------------------------------
Attachment: AVRO-1007.patch
Here's a version that just changes getDefault() to throw an exception when there's no default value and the field's type is not null. This is equivalent to the prior patch but with less generated code.
> Insufficient validation in generated specific record builder implementations
> ----------------------------------------------------------------------------
>
> Key: AVRO-1007
> URL: https://issues.apache.org/jira/browse/AVRO-1007
> Project: Avro
> Issue Type: Bug
> Affects Versions: 1.6.1
> Reporter: James Baldassari
> Assignee: James Baldassari
> Labels: java
> Fix For: 1.6.2
>
> Attachments: AVRO-1007-v2.patch, AVRO-1007-v3.patch, AVRO-1007-v4.patch, AVRO-1007.patch, AVRO-1007.patch
>
>
> The are two main problems with the generated build() method in specific record builders:
> * For non-primitive types, if there is no default value and the user does not set the value, build() will execute successfully without throwing an exception
> ** Instead, an AvroRuntimeException should be thrown with an exception message indicating the name of the required field that was not set
> * For primitive types, if there is no default value and the user does not set the value, an AvroRuntimeException is thrown with the 'cause' set to a NullPointerException, which is not very helpful
> ** The NPE comes from attempting to set the primitive field to the result of defaultValue(), which is null
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira