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:

- {{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

- all enum methods must throw {{BinaryObjectException}} in their signature to be consistent
with the rest API

- 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
- {{buildEnum(String typeName, String name)}} - NPE is possible if {{metadata0()}} returns
{{null}}, need to handle it properly

- {{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

- {{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

View raw message