ignite-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vladimir Ozerov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (IGNITE-4575) Implement in Ignite wrapper for enums based on H2 user value type
Date Fri, 05 May 2017 10:56:04 GMT

    [ 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)

Mime
View raw message