[
https://issues.apache.org/jira/browse/IGNITE-4575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15998143#comment-15998143
]
Vladimir Ozerov commented on IGNITE-4575:
-----------------------------------------
[~skalashnikov], I did partial review of binary enum part in
{{ignite-4575-binary}}. Please update the branch as I did minor changes there.
My comments:
{{BinaryTypeConfifuration}}
- {{setEnumValues(String... values)}} - method should be removed, map should be
enough; in future we will improve all Ignite API to support things like
"setSomething(Map)" + "addSomething(K key, V val)" + "clearSomething()"
- {{setEnumValues(Map<String, Integer> values)}} - setter should not modify
"isEnum". Let's follow a common rule that setter can mutate only one variable
{{IgniteBinary}}:
- all enum methods must throw {{BinaryObjectException}} in their signature to
be consistent with the rest API
CacheObjectBinaryProcessorImpl}}
- start()}} - please confirm that validation check that particular name has
unique ordinal; nothing else should be checked
- {{buildEnum(String typeName, int ord)}} - why did we remove
{{updateMetadata()}} call here? it should be legal to define an enum with
ordinals only; think of names as a convenient decorators around values;
- {{buildEnum(String typeName, String name)}} - {{BinaryObjectException}}
should be throw here
- {{buildEnum(String typeName, String name)}} - NPE is possible if
{{metadata0()}} returns {{null}}, need to handle it properly
{{BinaryObjectBuilderImpl}}
- {{checkMetadata()}} - I doubt that change from {{ENUM}} to {{BINARY_ENUM}} is
valid; if user set a enum in binary form, his normal expectation would be to
expect normal enum in deserialized form, but you write in metatada that it is
{{BINARY_ENUM}} instead
{{BinaryClassDescriptor}}
- {{isEnum()}} - looks wrong to me either; {{BinaryEnumObjectImpl}} is not enum
itself, it is something that encapsulates enum inside. As such, isEnum should
not return {{true}} in this case IMO.
> Implement in Ignite wrapper for enums based on H2 user value type
> -----------------------------------------------------------------
>
> Key: IGNITE-4575
> URL: https://issues.apache.org/jira/browse/IGNITE-4575
> Project: Ignite
> Issue Type: Task
> Components: SQL
> Reporter: Alexander Paschenko
> Assignee: Sergey Kalashnikov
> Labels: important
> Fix For: 2.1
>
>
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
