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/06 23:12:32 UTC
[jira] Created: (AVRO-185) specific should not depend on reflect
specific should not depend on reflect
-------------------------------------
Key: AVRO-185
URL: https://issues.apache.org/jira/browse/AVRO-185
Project: Avro
Issue Type: Improvement
Components: java
Reporter: Doug Cutting
Assignee: Doug Cutting
Fix For: 1.3.0
Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AVRO-185) specific should not depend on reflect
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-185:
------------------------------
Attachment: AVRO-185.patch
Here's a new version of the patch that
- fixes the javadoc
- moves reflection back to reflect
- removes paranamer from code generator in build.xml
Existing tests use reflection on generated test classes. Perhaps that ought to be changed, but until it is, we still run paranamer on generated test code to permit this.
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch, AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-185) specific should not depend on reflect
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776737#action_12776737 ]
Philip Zeyliger commented on AVRO-185:
--------------------------------------
+1 on the current patch.
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch, AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-185) specific should not depend on reflect
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776507#action_12776507 ]
Philip Zeyliger commented on AVRO-185:
--------------------------------------
Ah, yes, that makes sense. I think the javadoc in Specific.getProtocol() needs updating.
{noformat}
/** Generate a protocol for a Java interface.
* <p>Note that this requires that <a
* href="http://paranamer.codehaus.org/">Paranamer</a> is run over compiled
* interface declarations, since Java 6 reflection does not provide access to
* method parameter names. See Avro's build.xml for an example. </p>
*/
public Protocol getProtocol(Class iface) {
try {
return (Protocol)(iface.getDeclaredField("_PROTOCOL").get(null));
} catch (NoSuchFieldException e) {
throw new AvroRuntimeException(e);
} catch (IllegalAccessException e) {
throw new AvroRuntimeException(e);
}
}
{noformat}
Should you also update the build.xml to remove the paranamer compilation step?
-- Philip
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-185) specific should not depend on reflect
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776352#action_12776352 ]
Philip Zeyliger commented on AVRO-185:
--------------------------------------
This patch is a little tricky to follow, though most of it seems like moving code around. I assume that src/java/org/apache/avro/reflect/FixedSize.java is moved to src/java/org/apache/avro/specific/FixedSize.java as part of this patch?
I noticed that there's some support for nested classes (space = c.getEnclosingClass().getName() + "$"); That looked like new code; is there a small test with nested classes?
I'm confused why SpecificData.createSchema() uses reflection to generate the schema. "Specific" indicates that this uses the generated code, and the generated code has a schema object already there, waiting to be parsed. It seems like before this patch, there were two implementations of createSchema(), and there's only one left. Was that intentional?
Otherwise, everything looks good.
-- Philip
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (AVRO-185) specific should not depend on reflect
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776577#action_12776577 ]
Doug Cutting commented on AVRO-185:
-----------------------------------
> (space = c.getEnclosingClass().getName() + "$"); That looked like new code; is there a small test with nested classes?
That is not new code. And yes, TestReflect tests nested classes.
> I'm confused why SpecificData.createSchema() uses reflection to generate the schema.
Hmm. That javadoc should certainly remain in ReflectData. This method constructs entries for a cache from class->schema. At issue is whether you should be able to use GenericArray<Foo> as a top-level type, e.g., in a file, where Foo is defined by a SpecificRecord. If so, then SpecificData#createSchema() should support more than records and must use reflection for maps and arrays. That said, the logic copied from ReflectData currently handles some cases that the specific compiler doesn't generate, e.g., nested classes. And enum and fixed schemas are still created with reflection, when these should use the schema from the generated code.
> before this patch, there were two implementations of createSchema(), and there's only one left. Was that intentional?
Before SpecificData overrode ReflectData#createSchema() for the case of records. After, the record-specific logic is refactored to createRecordSchema() and ReflectData overrides that.
Perhaps I should replace this with createNamedSchema to handle records, enums and fixed, which differ between specific and reflect. I can also move the nested-class logic to reflect, plus fix the javadoc.
> Should you also update the build.xml to remove the paranamer compilation step?
Yes, good catch. We still need to run it for reflect tests, but not for generated code. Frankly, I can't now see where it's currently run on reflect tests... Hmm.
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AVRO-185) specific should not depend on reflect
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-185:
------------------------------
Status: Patch Available (was: Open)
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AVRO-185) specific should not depend on reflect
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-185:
------------------------------
Attachment: AVRO-185.patch
Here's a patch that implements this. Tests pass.
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (AVRO-185) specific should not depend on reflect
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/AVRO-185?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated AVRO-185:
------------------------------
Resolution: Fixed
Hadoop Flags: [Incompatible change, Reviewed] (was: [Incompatible change])
Status: Resolved (was: Patch Available)
I just committed this. Thanks for the reviews, Philip!
> specific should not depend on reflect
> -------------------------------------
>
> Key: AVRO-185
> URL: https://issues.apache.org/jira/browse/AVRO-185
> Project: Avro
> Issue Type: Improvement
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-185.patch, AVRO-185.patch
>
>
> Currently the specific implementation inherits from the reflect implementation. To fix AVRO-80, this dependency should be reversed, since specific will still share all data representations but records and enums with generic, but specific and reflect will no longer use common representations for strings and arrays.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.