[
https://issues.apache.org/jira/browse/IGNITE-14645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Kirill Tkalenko reassigned IGNITE-14645:
----------------------------------------
Assignee: Kirill Tkalenko (was: Ivan Bessonov)
> Support polymorphic configuration nodes.
> ----------------------------------------
>
> Key: IGNITE-14645
> URL: https://issues.apache.org/jira/browse/IGNITE-14645
> Project: Ignite
> Issue Type: Improvement
> Reporter: Ivan Bessonov
> Assignee: Kirill Tkalenko
> Priority: Major
> Labels: iep-55, ignite-3
>
> NOTE: description might not be finished.
> h3. Problem
> Currently configuration schema structure is very restricted and doesn't allow
> any variations in it. This approach comes with a set of problems.
> # How do you properly configure {{IpFinder}}? For each type of finder you
> only need a few properties.
> # How do you configure SQL indexes? Pretty much the same problem.
> h3. Interface
> For the solution we need to expand abilities of configuration schemas. I
> propose the following option:
> {code:java}
> // Configuration schema that contains polymorphic field.
> @Config
> class TableConfigurationSchema {
> @NamedConfigValue
> public IndexConfigurationSchema indexes;
> }
> // Base class for polymorphic value. Explicitly has all subclasses
> // in its description to simplify incremental code generation.
> //TODO: Maybe we cab avoid this part by using "originating elements" in
> annotation processing,
> // we'll check that during POC phase.
> @PolymorphicConfig(impl = {
> HashIndexConfigurationSchema.class,
> TreeIndexConfigurationSchema.class,
> })
> class IndexConfigurationSchema {
> // This annotation shows that current field defines implementation.
> // Specific values are present in implementations declarations.
> @Id
> @Immutable
> @Value
> public String type;
> }
> // One of implementations for index. Id value is defined in annotation.
> @PolymorphicInstance(id = "hash")
> public static class HashIndexConfigurationSchema extends
> IndexConfigurationSchema {
> @Immutable
> @Value
> public String column;
> }
> // Other implementation for index.
> @PolymorphicInstance(id = "tree")
> public static class TreeIndexConfigurationSchema extends
> IndexConfigurationSchema {
> @Immutable
> @Value
> public String[] columns;
> }
> {code}
> h3. Generated API
> We need to tweak API a little bit. I'd love to see as few changes as
> possible, so my vision is something like this:
> {code:java}
> TableConfiguration tableCfg = ...;
> tableCfg.indexes().create("hashIndexName", index ->
> // Id sets up automatically by the call.
> index.asHashIndex().changeColumn("columnName")
> ).get();
> tableCfg.indexes().update("hashIndexName", index ->
> // Any cast is allowed to do in change request.
> // But this update will fail during processing.
> index.asTreeIndex().changeColumns("a", "b")
> );
> IndexConfiguration indexCfg = tableCfg.indexes().get("hashIndexName");
> // This must be an instance of "HashIndexConfiguration".
> HashIndexConfiguration hashCfg = (HashIndexConfiguration)indexCfg;
> // This must be instance of HashIndexView,
> IndexView indexView = indexCfg.value();
> // Maybe this is redundant, I don't know.
> assert indexView.isHashIndex();
> // Returns the same object with a safe cast.
> // Maybe this is redundant as well and regular cast would be enough.
> HashIndexView hashView = indexView.asHashIndex();{code}
> h3. Implementation Notes
> There are quite a few places that need to be tweaked here. First of all,
> let's examine {{ConfigurationImpl}} classes:
>
> {code:java}
> // Let's assume for a moment that "index" is not a named list, this is more
> convenient for current example.
> final class TableConfigurationImpl extends DynamicConfiguration<TableView,
> TableChange> implements TableConfiguration {
> // Polymorphic fields won't be final anymore.
> private IndexConfigurationImpl index;
> public TableConfigurationImpl(List<String> prefix, String key, RootKey<?,
> ?> rootKey, ConfigurationChanger changer) {
> super(prefix, key, rootKey, changer);
> // No more "add" method invocations. "members" becomes mutable
> collections.
> // There's no point in making specific code generation for the
> situation when it's static.
> }
> @Override public final IndexConfiguration index() {
> // It's important to note that we can't just read field content
> because its type might have been changed.
> // This scenario was impossible before polymorphic types.
> refreshValue();
> return index;
> }
> @Override protected final synchronized void beforeRefreshValue(IndexView
> newValue) {
> // New value must be reinstantiated if its type changed.
> }
> @Override public Map<String, ConfigurationProperty<?, ?>> members() {
> refreshValue();
> // Return map computed from current fields OR throw this method away
> completely and replace it
> // with proper "getMember". BTW, NamedListConfiguration would need
> "getAllNames" method or something close to it.
> // So inner nodes and named list nodes will probably only use one of
> these, we might put different methods
> // into different classes.
> }
> }{code}
> It's important to note that {{refreshValue}} method will work differently.
> You can't just use {{find}} because it doesn't know anything about
> polymorphism. We will need a new specific {{find}} for this case that will
> check every polymorphic type in chain. It's pretty easy to implement tho.
>
> Now nodes. It's a bit more complicated.
> {code:java}
> final class TableNode extends InnerNode implements TableView, TableChange {
> private final TableConfigurationSchema _spec = new ...;
> private IndexNode index;
> // This one is the same.
> @Override public IndexNode changeIndex(Consumer<IndexChange>
> indexConsumer) {
> ...
> }
> // But this one is not.
> @Override public IndexView index() {
> // We need to downcast view object here.
> return index.specificView();
> }
> // Traverse methods remain intact.
> @Override public void construct(String key, ConfigurationSource src)
> throws NoSuchElementException {
> switch (key) {
> case "index":
> if (src == null) child = null;
> // Creating new node.
> else if (child == null) {
> switch (src.readMandatoryProperty("type")) {
> case "hash":
> src.descend(index = new IndexNode("hash"));
> break;
> case "tree":
> src.descend(index = new IndexNode("tree"));
> break;
> default:
> throw new ...;
> }
> }
> // Updating existing node.
> else {
> String id = src.readMandatoryProperty("type");
> if (id.equals(index.type()))
> src.descend(index = (IndexNode)index.copy());
> else {
> // Copy-paste. This can probably be compacted, I just
> don't want to mess with code
> // complexity in example.
> switch (id) {
> case "hash":
> src.descend(index = new IndexNode("hash"));
> // Or copy+setType?
> break;
> case "tree":
> src.descend(index = new IndexNode("tree"));
> break;
> default:
> throw new ...;
> }
> }
> }
> break;
> default: throw new NoSuchElementException(key);
> }
> }
> }{code}
> Index node. All types of names collisions are allowed.
> {code:java}
> // This is already different enough. We have no "View" interface and extended
> "Change" interface.
> // Second one will contain methods like "asHashIndex".
> final class IndexNode extends InnerNode implements IndexChangeEx {
> // Specs are lazy now.
> private HashIndexConfigurationSchema _spec$HashIndex;
> private TreeIndexConfigurationSchema _spec$TreeIndex;
> // Inlined fields.
> private String type;
> private String column$HashIndex;
> private String[] columns$TreeIndex;
> // Constructors...
> // New methods:
> public IndexView specificView() {
> switch (type) {
> case "hash": return this.new HashIndexNode(); // Inner class,
> described later.
> case "tree": return this.new TreeIndexNode();
> default: throw new ...;
> }
> }
> // This methods is required to transform Ex change to a specific one.
> @Override public HashIndexChange asHashIndex() {
> if (type == null)
> type = "hash";
> else if (!"hash".equals(type))
> throw new ...;
> return this.new HashNode();
> }
> @Override public <T> void traverseChildren(ConfigurationVisitor<T>
> visitor) {
> visitor.visitInnerNode("type", type);
> switch (type) {
> case "hash":
> visitor.visitLeafNode("column", column$HashIndex);
> break;
> case "tree":
> visitor.visitLeafNode("columns", columns$TreeIndex);
> break;
> }
> }
> @Override public void construct(String key, ConfigurationSource src)
> throws NoSuchElementException {
> switch (key) {
> case "type":
> assert src != null;
> index = src.unwrap(String.class);
> // Validation - switch by all known types.
> break;
> }
> switch (type) {
> case "hash":
> switch (key) {
> case "column":
> column$HashIndex = src == null ? null :
> src.unwrap(String.class);
> break;
> default: throw new NoSuchElementException(key);
> }
> break;
> case "tree":
> ...
> break;
> default:
> throw new ...;
> }
> }
> @Override public Class<?> schemaType() {
> switch (type) {
> case "hash":
> // I know that we could just return class. Such code is
> required right now so that spec fields
> // won't be unused during compilation. Bad reasoning, maybe
> we'll fix it.
> if (_spec$HashIndex == null) _spec$HashIndex = new
> HashIndexConfigurationSchema();
> return _spec$HashIndex.getClass();
> ...
> }
> }
> // Same for constructDefault, you get the idea.
> // Finally, private inner classes for specific change/view
> private class HashIndexNode implements HashIndexView, HashIndexChange {
> // And those implement IndexView and IndexChange.
> // All view and change methods will delegate to the fields of
> IndexNode,
> // validating "type" field at the same time.
> // No traversing / constructing here, everything's simple.
> }
> }{code}
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
