You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "Doug Cutting (JIRA)" <ji...@apache.org> on 2009/11/25 23:51:40 UTC

[jira] Created: (AVRO-242) add support for per-schema metadata

add support for per-schema metadata
-----------------------------------

                 Key: AVRO-242
                 URL: https://issues.apache.org/jira/browse/AVRO-242
             Project: Avro
          Issue Type: New Feature
          Components: java
            Reporter: Doug Cutting
            Assignee: Doug Cutting
             Fix For: 1.3.0


The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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


[jira] Resolved: (AVRO-242) add support for per-schema metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting resolved AVRO-242.
-------------------------------

    Resolution: Fixed

I just committed this.

> add support for per-schema metadata
> -----------------------------------
>
>                 Key: AVRO-242
>                 URL: https://issues.apache.org/jira/browse/AVRO-242
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>         Attachments: AVRO-242.patch, AVRO-242.patch
>
>
> The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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


[jira] Commented: (AVRO-242) add support for per-schema metadata

Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12782690#action_12782690 ] 

Philip Zeyliger commented on AVRO-242:
--------------------------------------

The avro specification doesn't indicate that the values must be Strings, so they could be arbitrary JSON.  Not sure what the best approach is, though, from a Java API.

> add support for per-schema metadata
> -----------------------------------
>
>                 Key: AVRO-242
>                 URL: https://issues.apache.org/jira/browse/AVRO-242
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>
> The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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


[jira] Commented: (AVRO-242) add support for per-schema metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12784386#action_12784386 ] 

Doug Cutting commented on AVRO-242:
-----------------------------------

My thinking is that the Java API would initially only get/set string-based values.  It should probably throw an exception when you attempt to read a string that has a non-string, non-empty value.

> add support for per-schema metadata
> -----------------------------------
>
>                 Key: AVRO-242
>                 URL: https://issues.apache.org/jira/browse/AVRO-242
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>
> The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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


[jira] Commented: (AVRO-242) add support for per-schema metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12782664#action_12782664 ] 

Doug Cutting commented on AVRO-242:
-----------------------------------

The API might be something like:

public String getProperty(String name);
public Schema addProperty(String name, String value);

Adding a value creates and returns new schema with the new value.  The original schema remains unmodified.

> add support for per-schema metadata
> -----------------------------------
>
>                 Key: AVRO-242
>                 URL: https://issues.apache.org/jira/browse/AVRO-242
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>
> The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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


[jira] Commented: (AVRO-242) add support for per-schema metadata

Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12785142#action_12785142 ] 

Philip Zeyliger commented on AVRO-242:
--------------------------------------

Overall looks good.

I think the code would be simpler (fewer null checks, equalProps & propsHash wouldn't be necessary) if you didn't mind creating an extra HashMap per Schema.  That is, just start with an empty map.

bq. Iterator<String> i = schema.getFieldNames();

Do you dislike the Java 1.5 for syntax?  You can put the two lines you save towards your braces account.  (<ducks>)

bq. setProp()/getProp

For a public API, I have a mild preference for setProperty() and getProperty().  I think that's the API you suggested before.

bq. schema.get(prop).getTextValue()

There's sometimes not going to be a text value.  Is there a test for that?  You should, presumably, just skip those properties.

bq. static final Map<String,Type> PRIMITIVES = new HashMap<String,Type>();

Unrelated to the current issue, but since you're changing these lines... You could write this as:

for (Type t : Type.somethingOrOther()) {
  if (t.isPrimitive()) {
    PRIMITIVES.add(type.name, type);
  }
}

You'd need to update the Type enum to have a boolean for whether things are primitive or not, which seems useful anyway.

It seems visually useful to put the PRIMITIVES list and the Type enum closer together in the java file.

> add support for per-schema metadata
> -----------------------------------
>
>                 Key: AVRO-242
>                 URL: https://issues.apache.org/jira/browse/AVRO-242
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>         Attachments: AVRO-242.patch
>
>
> The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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


[jira] Updated: (AVRO-242) add support for per-schema metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting updated AVRO-242:
------------------------------

    Attachment: AVRO-242.patch

Here's a patch that implements this.

> add support for per-schema metadata
> -----------------------------------
>
>                 Key: AVRO-242
>                 URL: https://issues.apache.org/jira/browse/AVRO-242
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>         Attachments: AVRO-242.patch
>
>
> The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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


[jira] Updated: (AVRO-242) add support for per-schema metadata

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting updated AVRO-242:
------------------------------

    Attachment: AVRO-242.patch

> I think the code would be simpler (fewer null checks, equalProps & propsHash
> wouldn't be necessary) if you didn't mind creating an extra HashMap per Schema.

Yeah, I guess so.  I was not worried about the size of schema instances, but rather about the speed of parsing and constructing schemas.  This patch already affects that a lot, since primitive schemas are no longer constants, and I'd hoped not make it worse yet by constructing a HashMap for each primitive in a schema.  But, as you argued elsewhere, in cases where this is a cost (e.g., per file block) we can usually probably arrange to cache schema instances.

I've changed this in the attached patch.

> Do you dislike the Java 1.5 for syntax?

It's not applicable here.  The iterator() method enumerates values, but we want keys.  JsonNode has no keySet() or entrySet() method.

> There's sometimes not going to be a text value. Is there a test for that?

I added one.


> add support for per-schema metadata
> -----------------------------------
>
>                 Key: AVRO-242
>                 URL: https://issues.apache.org/jira/browse/AVRO-242
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Doug Cutting
>            Assignee: Doug Cutting
>             Fix For: 1.3.0
>
>         Attachments: AVRO-242.patch, AVRO-242.patch
>
>
> The Avro specification permits schema json to contain other property names to be used besides those listed in the specification.  I propose adding support to Java for getting and setting these.

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