Re: Review Request 30327: Drill-1545 v2 allowing custom extensions on json files

[Jason Altekruse <al...@gmail.com>]

> On Feb. 23, 2015, 1:21 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONFormatPlugin.java, line 91
> > <https://reviews.apache.org/r/30327/diff/1/?file=837068#file837068line91>
> >
> >     Can you please use @JsonInclude(Include.NON_DEFAULT)
> >     
> >     Wouldn't that make this backwards compatible?
> 
> Jason Altekruse wrote:
>     Looking at the docs I think that Include.NON_DEFAULT only works on primitives. It doesn't specifically mention lists, but it does metnion that it will always include a map. I think the safest way is just to create two separate methods, one specifically to check if we have only the default member of the list and in this case return null, this will only be use for Jackson (We will also have to turn off populating nulls, I think it is currently turned on). There will be a separate method that returns the list even if it is the default, for use in the other parts of the code.

I was wrong about this, the annotation ended up fixing the case for the list, I confirmed that it does not serialize the value if it is the default list. I also confirmed that an old version of Drill could be started with a zookeeper config left over from a version including this patch (where the default value had not been changed, so it was omitted).


- Jason


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30327/#review73481
-----------------------------------------------------------


On Jan. 27, 2015, 9:56 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30327/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 9:56 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-1545
>     https://issues.apache.org/jira/browse/DRILL-1545
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Patch add support for custom extensions on json files. There was a previous review request for this issue, but there were some problems while testing the patch in a shared cluster environment and with compressed json files. I was not the owner of the previous review request so I could not upload this there.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/BasicFormatMatcher.java 2ba2910 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONFormatPlugin.java d41243d 
>   exec/java-exec/src/main/resources/bootstrap-storage-plugins.json 6bf1872 
> 
> Diff: https://reviews.apache.org/r/30327/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>