[
https://issues.apache.org/jira/browse/IGNITE-28907?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Oleg Valuyskiy updated IGNITE-28907:
------------------------------------
Description:
h2. Problem
{\{CacheConfiguration.setIndexedTypes(...)}} and
\{{CacheConfiguration.setQueryEntities(...)}} both populate the same internal
collection of \{{QueryEntity}} definitions.
When both methods configure the same value type, the current implementation
treats the second \{{QueryEntity}} as a duplicate based only on its value type
and silently ignores it.
As a result, SQL metadata supplied by the second configuration method is lost.
The issue is not specific to annotation-based indexes.
{\{setIndexedTypes(...)}} creates a \{{QueryEntity}} for every configured
key/value type pair even when the value class does not contain any
\{{@QuerySqlField}} annotations. Therefore, merely configuring a value type
through \{{setIndexedTypes(...)}} is enough to prevent a subsequent
\{{QueryEntity}} for the same value type from being applied.
For example:
{code:java}
CacheConfiguration<Integer, Person> ccfg =
new CacheConfiguration<>("person-cache");
ccfg.setIndexedTypes(Integer.class, Person.class);
ccfg.setQueryEntities(Collections.singletonList(
new QueryEntity()
.setKeyType(Integer.class.getName())
.setValueType(Person.class.getName())
.setFields(fields)
.setIndexes(Collections.singletonList(
new QueryIndex(Arrays.asList("name", "age"))
.setName("PERSON_NAME_AGE_IDX")
))
));
{code}
Even if \{{Person}} has no query annotations, \{{setIndexedTypes(...)}} creates
a \{{QueryEntity}} for \{{Person}} first.
The explicit \{{QueryEntity}} supplied later through \{{setQueryEntities(...)}}
has the same value type and is silently skipped.
The cache starts successfully, but \{{PERSON_NAME_AGE_IDX}} is not created.
If \{{Person}} additionally contains:
{code:java}
@QuerySqlField(index = true)
private String name;
{code}
the annotation-derived single-column index is created, while the explicitly
configured composite index is still missing.
Therefore, the actual conflict is between two \{{CacheConfiguration}} APIs:
* {\{setIndexedTypes(...)}};
* {\{setQueryEntities(...)}};
rather than between annotations and explicit configuration as such.
h2. Root cause
Both methods store query metadata in the same internal \{{qryEntities}}
collection.
The existing behavior is conceptually equivalent to:
{code:java}
if (!containsQueryEntityWithSameValueType(newEntity))
qryEntities.add(newEntity);
{code}
If a \{{QueryEntity}} with the same value type is already present:
* the entities are not merged;
* fields are not compared;
* indexes are not merged;
* aliases and constraints are not merged;
* conflicting metadata is not detected;
* the incoming entity is silently ignored.
This makes the resulting SQL schema incomplete and dependent on the order in
which configuration methods are invoked.
h2. Expected behavior
When \{{setIndexedTypes(...)}} and \{{setQueryEntities(...)}} configure the
same value type, Ignite should attempt to merge the corresponding
\{{QueryEntity}} definitions.
Compatible metadata should be combined.
Conflicting metadata should result in a \{{CacheException}} instead of silently
selecting one definition.
The merge should be incremental and operate on the effective, already
accumulated \{{QueryEntity}}.
Conceptually:
{code:java}
QueryEntity existing = findQueryEntity(valueType);
if (existing == null)
qryEntities.add(incoming);
else
replaceQueryEntity(existing, mergeQueryEntities(existing, incoming));
{code}
This allows multiple complementary calls to \{{setQueryEntities(...)}} to
accumulate metadata instead of losing information from previous calls.
h2. Merge rules
The following merge semantics should be applied.
h3. Scalar properties
For properties such as:
* key type;
* value type;
* table name;
* key field name;
* value field name;
the rules are:
{noformat}
null + X -> X
X + null -> X
X + X -> X
X + Y -> CacheException
{noformat}
h3. Fields
Field definitions should be merged while preserving the order of the existing
entity.
Fields present only in the incoming entity should be appended.
For the same field:
* equal field types are compatible;
* different field types must cause a \{{CacheException}}.
Example:
{noformat}
existing:
name : String
age : Integer
incoming:
age : Integer
city : String
result:
name : String
age : Integer
city : String
{noformat}
h3. Indexes
Indexes with different names should be combined.
For indexes with the same name:
* identical definitions should be deduplicated;
* different definitions must cause a \{{CacheException}}.
The comparison must take the complete index definition into account, including:
* indexed fields;
* field order;
* ascending/descending order;
* index type;
* inline size where applicable.
h3. Map-based metadata
Metadata such as:
* aliases;
* default field values;
* field precision;
* field scale;
should be merged by key.
For the same key:
* equal values are compatible;
* different values must cause a \{{CacheException}}.
h3. Set-based metadata
Metadata such as:
* key fields;
* not-null fields;
should be merged using set union.
h2. Multiple setQueryEntities calls
The implementation should support consecutive complementary calls to
\{{setQueryEntities(...)}}.
For example:
{code:java}
ccfg.setQueryEntities(Collections.singletonList(entityWithNameIndex));
ccfg.setQueryEntities(Collections.singletonList(entityWithAgeIndex));
ccfg.setQueryEntities(Collections.singletonList(entityWithCompositeIndex));
{code}
The resulting \{{QueryEntity}} must contain metadata from all three calls.
The merge must use the current effective entity rather than reconstructing the
original entity from \{{indexedTypes}}, otherwise metadata accumulated by
previous merge operations can be lost.
h2. setIndexedTypes state
{\{setIndexedTypes(...)}} creates a boxed copy of the supplied key/value type
array, but the resulting array must also be stored in the \{{indexedTypes}}
field.
The field is already exposed through \{{getIndexedTypes()}} and is also used to
prevent repeated \{{setIndexedTypes(...)}} calls.
The configuration should therefore preserve the successfully applied indexed
types:
{code:java}
this.indexedTypes = newIndexedTypes;
{code}
The assignment should happen only after successful processing of the supplied
types.
h2. Expected result
For the following configuration:
{code:java}
ccfg.setIndexedTypes(Integer.class, Person.class);
ccfg.setQueryEntities(Collections.singletonList(
personEntityWithCompositeIndex()
));
{code}
the resulting SQL schema should contain the explicitly configured composite
index even if \{{Person}} has no query annotations.
If \{{Person}} also declares:
{code:java}
@QuerySqlField(index = true)
private String name;
{code}
both indexes should be created:
{noformat}
PERSON_NAME_IDX
PERSON_NAME_AGE_IDX
{noformat}
along with the default primary-key index.
h2. Acceptance criteria
* {\{setIndexedTypes(...)}} followed by \{{setQueryEntities(...)}} merges
compatible metadata for the same value type.
* {\{setQueryEntities(...)}} followed by \{{setIndexedTypes(...)}} also merges
compatible metadata.
* Multiple complementary \{{setQueryEntities(...)}} calls accumulate metadata.
* Different value types remain separate \{{QueryEntity}} definitions.
* Fields with the same name and type are deduplicated.
* Fields with the same name and different types cause a \{{CacheException}}.
* Different indexes are combined.
* Identical indexes with the same name are deduplicated.
* Indexes with the same name but different definitions cause a
\{{CacheException}}.
* Compatible aliases, precision, scale, defaults, key fields and not-null
fields are merged.
* Conflicting scalar or map-based metadata causes a \{{CacheException}}.
* Annotation-derived metadata created through \{{setIndexedTypes(...)}} can be
combined with explicitly configured metadata from \{{setQueryEntities(...)}}.
* SQL metadata is no longer silently discarded based only on duplicate value
type.
* {\{getIndexedTypes()}} returns the types successfully configured through
\{{setIndexedTypes(...)}}.
* Integration tests verify that merged index definitions are actually
registered in the SQL schema and exposed through the \{{INDEXES}} system view.
was:
h2. Problem
Apache Ignite allows the same SQL value type to be configured simultaneously
through:
* {*}CacheConfiguration#setIndexedTypes{*}, which creates a *QueryEntity* from
annotations such as *@QuerySqlField*
* {*}CacheConfiguration#setQueryEntities{*}, which supplies a *QueryEntity*
explicitly configured in the node configuration xml-file
Ignite does not support merging these two query entity definitions. When both
configuration mechanisms describe the same value type, the query entity
registered first is retained, while the subsequently supplied entity is ignored
without any warning. The node still starts successfully and the cache remains
operational, but part of the configured SQL schema may be missing.
For example, single-field indexes may be declared using {*}@QuerySqlField(index
= true){*}, while a composite index for the same value type is configured
through *queryEntities* in the node configuration. Depending on the order in
which the corresponding setters are invoked, either the annotation-derived
indexes or the explicitly configured indexes are NOT created.
Reproducer: [^MixedIndexConfigurationReproducer.patch]
Starting a node with such a partially applied configuration is unsafe. The
configuration error must be detected before the cache is started.
h2. Root cause
Both *CacheConfiguration#setIndexedTypes* and
*CacheConfiguration#setQueryEntities* store query metadata in the same internal
*qryEntities* collection. When an entity with the same value type is already
present, the incoming entity is treated as a duplicate and is not added (see
{*}CacheConfiguration#setQueryEntities{*}):
{code:java}
for (QueryEntity entity : qryEntities) {
boolean found = false;
for (QueryEntity existing : this.qryEntities) {
if (Objects.equals(entity.findValueType(), existing.findValueType())) {
found = true;
break;
}
}
if (!found)
this.qryEntities.add(entity);
}{code}
When the value type is already registered:
* the existing and incoming entities are not merged
* their fields and indexes are not compared
* conflicting or additional metadata is not validated
* the ignored configuration is not reported
The first registered query entity effectively wins.
h2. Expected behavior
Ignite must reject a cache configuration in which the same value type is
configured through both *indexedTypes* and {*}queryEntities{*}. The node must
fail to start with a *CacheException* instead of silently ignoring one of the
query entity definitions. The validation must not depend on the order in which
the configuration setters are called.
h2. Possible follow-up
Support for combining compatible query entity definitions may be considered as
a separate improvement. Such an improvement would require explicit merge and
conflict-resolution rules for:
* fields and field types
* table names
* aliases
* key fields
* index names and definitions
* other QueryEntity metadata
Until such rules are defined and implemented, fail-fast validation is prefered
as opposed to starting a node with an incomplete SQL schema.
> Support merging QueryEntity metadata configured through setIndexedTypes and
> setQueryEntities
> --------------------------------------------------------------------------------------------
>
> Key: IGNITE-28907
> URL: https://issues.apache.org/jira/browse/IGNITE-28907
> Project: Ignite
> Issue Type: Task
> Reporter: Oleg Valuyskiy
> Assignee: Oleg Valuyskiy
> Priority: Major
> Labels: ise
> Attachments: MixedIndexConfigurationReproducer.patch
>
>
> h2. Problem
> {\{CacheConfiguration.setIndexedTypes(...)}} and
> \{{CacheConfiguration.setQueryEntities(...)}} both populate the same internal
> collection of \{{QueryEntity}} definitions.
> When both methods configure the same value type, the current implementation
> treats the second \{{QueryEntity}} as a duplicate based only on its value
> type and silently ignores it.
> As a result, SQL metadata supplied by the second configuration method is lost.
> The issue is not specific to annotation-based indexes.
> {\{setIndexedTypes(...)}} creates a \{{QueryEntity}} for every configured
> key/value type pair even when the value class does not contain any
> \{{@QuerySqlField}} annotations. Therefore, merely configuring a value type
> through \{{setIndexedTypes(...)}} is enough to prevent a subsequent
> \{{QueryEntity}} for the same value type from being applied.
> For example:
> {code:java}
> CacheConfiguration<Integer, Person> ccfg =
> new CacheConfiguration<>("person-cache");
> ccfg.setIndexedTypes(Integer.class, Person.class);
> ccfg.setQueryEntities(Collections.singletonList(
> new QueryEntity()
> .setKeyType(Integer.class.getName())
> .setValueType(Person.class.getName())
> .setFields(fields)
> .setIndexes(Collections.singletonList(
> new QueryIndex(Arrays.asList("name", "age"))
> .setName("PERSON_NAME_AGE_IDX")
> ))
> ));
> {code}
> Even if \{{Person}} has no query annotations, \{{setIndexedTypes(...)}}
> creates a \{{QueryEntity}} for \{{Person}} first.
> The explicit \{{QueryEntity}} supplied later through
> \{{setQueryEntities(...)}} has the same value type and is silently skipped.
> The cache starts successfully, but \{{PERSON_NAME_AGE_IDX}} is not created.
> If \{{Person}} additionally contains:
> {code:java}
> @QuerySqlField(index = true)
> private String name;
> {code}
> the annotation-derived single-column index is created, while the explicitly
> configured composite index is still missing.
> Therefore, the actual conflict is between two \{{CacheConfiguration}} APIs:
> * {\{setIndexedTypes(...)}};
> * {\{setQueryEntities(...)}};
> rather than between annotations and explicit configuration as such.
> h2. Root cause
> Both methods store query metadata in the same internal \{{qryEntities}}
> collection.
> The existing behavior is conceptually equivalent to:
> {code:java}
> if (!containsQueryEntityWithSameValueType(newEntity))
> qryEntities.add(newEntity);
> {code}
> If a \{{QueryEntity}} with the same value type is already present:
> * the entities are not merged;
> * fields are not compared;
> * indexes are not merged;
> * aliases and constraints are not merged;
> * conflicting metadata is not detected;
> * the incoming entity is silently ignored.
> This makes the resulting SQL schema incomplete and dependent on the order in
> which configuration methods are invoked.
> h2. Expected behavior
> When \{{setIndexedTypes(...)}} and \{{setQueryEntities(...)}} configure the
> same value type, Ignite should attempt to merge the corresponding
> \{{QueryEntity}} definitions.
> Compatible metadata should be combined.
> Conflicting metadata should result in a \{{CacheException}} instead of
> silently selecting one definition.
> The merge should be incremental and operate on the effective, already
> accumulated \{{QueryEntity}}.
> Conceptually:
> {code:java}
> QueryEntity existing = findQueryEntity(valueType);
> if (existing == null)
> qryEntities.add(incoming);
> else
> replaceQueryEntity(existing, mergeQueryEntities(existing, incoming));
> {code}
> This allows multiple complementary calls to \{{setQueryEntities(...)}} to
> accumulate metadata instead of losing information from previous calls.
> h2. Merge rules
> The following merge semantics should be applied.
> h3. Scalar properties
> For properties such as:
> * key type;
> * value type;
> * table name;
> * key field name;
> * value field name;
> the rules are:
> {noformat}
> null + X -> X
> X + null -> X
> X + X -> X
> X + Y -> CacheException
> {noformat}
> h3. Fields
> Field definitions should be merged while preserving the order of the existing
> entity.
> Fields present only in the incoming entity should be appended.
> For the same field:
> * equal field types are compatible;
> * different field types must cause a \{{CacheException}}.
> Example:
> {noformat}
> existing:
> name : String
> age : Integer
> incoming:
> age : Integer
> city : String
> result:
> name : String
> age : Integer
> city : String
> {noformat}
> h3. Indexes
> Indexes with different names should be combined.
> For indexes with the same name:
> * identical definitions should be deduplicated;
> * different definitions must cause a \{{CacheException}}.
> The comparison must take the complete index definition into account,
> including:
> * indexed fields;
> * field order;
> * ascending/descending order;
> * index type;
> * inline size where applicable.
> h3. Map-based metadata
> Metadata such as:
> * aliases;
> * default field values;
> * field precision;
> * field scale;
> should be merged by key.
> For the same key:
> * equal values are compatible;
> * different values must cause a \{{CacheException}}.
> h3. Set-based metadata
> Metadata such as:
> * key fields;
> * not-null fields;
> should be merged using set union.
> h2. Multiple setQueryEntities calls
> The implementation should support consecutive complementary calls to
> \{{setQueryEntities(...)}}.
> For example:
> {code:java}
> ccfg.setQueryEntities(Collections.singletonList(entityWithNameIndex));
> ccfg.setQueryEntities(Collections.singletonList(entityWithAgeIndex));
> ccfg.setQueryEntities(Collections.singletonList(entityWithCompositeIndex));
> {code}
> The resulting \{{QueryEntity}} must contain metadata from all three calls.
> The merge must use the current effective entity rather than reconstructing
> the original entity from \{{indexedTypes}}, otherwise metadata accumulated by
> previous merge operations can be lost.
> h2. setIndexedTypes state
> {\{setIndexedTypes(...)}} creates a boxed copy of the supplied key/value type
> array, but the resulting array must also be stored in the \{{indexedTypes}}
> field.
> The field is already exposed through \{{getIndexedTypes()}} and is also used
> to prevent repeated \{{setIndexedTypes(...)}} calls.
> The configuration should therefore preserve the successfully applied indexed
> types:
> {code:java}
> this.indexedTypes = newIndexedTypes;
> {code}
> The assignment should happen only after successful processing of the supplied
> types.
> h2. Expected result
> For the following configuration:
> {code:java}
> ccfg.setIndexedTypes(Integer.class, Person.class);
> ccfg.setQueryEntities(Collections.singletonList(
> personEntityWithCompositeIndex()
> ));
> {code}
> the resulting SQL schema should contain the explicitly configured composite
> index even if \{{Person}} has no query annotations.
> If \{{Person}} also declares:
> {code:java}
> @QuerySqlField(index = true)
> private String name;
> {code}
> both indexes should be created:
> {noformat}
> PERSON_NAME_IDX
> PERSON_NAME_AGE_IDX
> {noformat}
> along with the default primary-key index.
> h2. Acceptance criteria
> * {\{setIndexedTypes(...)}} followed by \{{setQueryEntities(...)}} merges
> compatible metadata for the same value type.
> * {\{setQueryEntities(...)}} followed by \{{setIndexedTypes(...)}} also
> merges compatible metadata.
> * Multiple complementary \{{setQueryEntities(...)}} calls accumulate
> metadata.
> * Different value types remain separate \{{QueryEntity}} definitions.
> * Fields with the same name and type are deduplicated.
> * Fields with the same name and different types cause a \{{CacheException}}.
> * Different indexes are combined.
> * Identical indexes with the same name are deduplicated.
> * Indexes with the same name but different definitions cause a
> \{{CacheException}}.
> * Compatible aliases, precision, scale, defaults, key fields and not-null
> fields are merged.
> * Conflicting scalar or map-based metadata causes a \{{CacheException}}.
> * Annotation-derived metadata created through \{{setIndexedTypes(...)}} can
> be combined with explicitly configured metadata from
> \{{setQueryEntities(...)}}.
> * SQL metadata is no longer silently discarded based only on duplicate value
> type.
> * {\{getIndexedTypes()}} returns the types successfully configured through
> \{{setIndexedTypes(...)}}.
> * Integration tests verify that merged index definitions are actually
> registered in the SQL schema and exposed through the \{{INDEXES}} system view.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
