[ 
https://issues.apache.org/jira/browse/AVRO-251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789825#action_12789825
 ] 

Philip Zeyliger commented on AVRO-251:
--------------------------------------

I haven't had a chance to fully grok the changes to Schema.java yet 
(particularly the writeSchema() code), but here's what I've been reviewing so 
far...


bq. I wonder if we even need to serialize default values.

It depends on what the goal of a binary schema is.  If we want to preserve the 
entire nuance of the writing schema, we should keep the defaults.  In protocol 
buffers, it's sometimes the case that the default values aren't materialized, 
because a missing value gets read as the default (this assumes same read and 
write schema).  That's not the case here: it's actually valid to evolve a 
schema by changing the default value (this is not really valid in PBs).

bq. Schema.Field.equals() to use string comparison of default values

This deserves a quick comment in the code.  It's nuanced enough that someone 
might trip up over it later.

bq. names.space(savedSpace);                  // restore space

I suspect this was a bug before (not popping back the previous space).  I 
didn't see a test for it.

bq. avro/Schema.m4

I should finish off AVRO-152 so there could be comments in here.

One thing I noticed is that you don't store "other" fields.  For example, in 
AVRO-152 I introduce adding "doc" to records and protocols.  And, of course, 
you've been using the ability to write arbitrary extra fields to implement the 
reflection API.  I could see either storing or not storing extra stuff; I think 
it's quite similar to the question of whether or not to store defaults.

Do you think it would be useful to have a hook at the schema type to have a way 
to evolve it?  Should it be a union?

bq. src/schemata/org/json/Json.avsc: {"type": "record", "name": "Field",

I think you should call this "object" to follow the JSON terminology.  See 
http://www.json.org/fatfree.html .

I'm also not sure about putting this into "org.json".  "org.apache.avro.json", 
perhaps?

bq.  //System.out.println(new String(data, "UTF-8"));

Ditch it?



> add schema for schemas
> ----------------------
>
>                 Key: AVRO-251
>                 URL: https://issues.apache.org/jira/browse/AVRO-251
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>         Attachments: AVRO-251.patch, AVRO-251.patch
>
>
> A schema for schemas would permits schemas to be written in binary.

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

Reply via email to