[
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. Reproducer: [^MixedIndexConfigurationTest.patch]
h2. Root cause
Both methods store query metadata in the same internal {{qryEntities}}
collection. 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{}}}.
h2. Merge rules
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. Duplicate value class handling
The existing {{DuplicateKeyValueClassesSelfTest#testDuplicateValueClass}}
covered the following configuration:
{code:java}
setIndexedTypes(
UUID.class, Clazz1.class,
String.class, Clazz1.class
);
{code}
The test expected the cache to start successfully. However, the previous
implementation did not actually preserve both key-value mappings.
{{CacheConfiguration#setIndexedTypes(...)}} creates a {{QueryEntity}} for each
key-value pair and identifies duplicates by value type. When the second pair
with the same value type was encountered, its {{QueryEntity}} was silently
discarded. As a result, the test effectively verified the same silent metadata
loss that this change is intended to eliminate. Reproducer:
[^DuplicateKeyValueClassesSelfTest.patch]
The test is therefore changed to expect a {{CacheException}} for the
conflicting key types instead of successful cache startup.
This does not affect the supported case where the same key class is used with
different value classes. For example:
{code:java}
setIndexedTypes(
UUID.class, Clazz1.class,
UUID.class, Clazz2.class
);
{code}
still produces two independent query entities because their value types are
different.
h2. IgnitePdsCorruptedIndexTest adjustment
{\{IgnitePdsCorruptedIndexTest#testCorruption}} previously configured indexed
types as follows:
{code}
.setIndexedTypes(
Integer.class, IndexedObject.class,
Long.class, IndexedObject.class
)
{code}
Both pairs use the same value type but different key types. With the previous
\{{setIndexedTypes(...)}} implementation, the second \{{QueryEntity}} was
detected as a duplicate by value type and silently discarded. Therefore, the
effective cache configuration contained only:
{code}
Integer -> IndexedObject
{code}
The \{{Long -> IndexedObject}} mapping never participated in the actual test
scenario.
The purpose of \{{IgnitePdsCorruptedIndexTest}} is to verify index
consistency/recovery after persistent partition data is deliberately corrupted
and the cluster is restarted. The test inserts entries with \{{Integer}} keys
and uses indexes defined on fields of \{{IndexedObject}}. The discarded \{{Long
-> IndexedObject}} configuration has no effect on this scenario.
The test configuration is therefore changed to:
{code}
.setIndexedTypes(Integer.class, IndexedObject.class)
{code}
This only removes metadata that was already ignored by the previous
implementation and does not change the behavior or coverage of the
corruption/recovery scenario itself.
was:
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. Reproducer: [^MixedIndexConfigurationTest.patch]
h2. Root cause
Both methods store query metadata in the same internal {{qryEntities}}
collection. 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{}}}.
h2. Merge rules
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. Duplicate value class handling
The existing {{DuplicateKeyValueClassesSelfTest#testDuplicateValueClass}}
covered the following configuration:
{code:java}
setIndexedTypes(
UUID.class, Clazz1.class,
String.class, Clazz1.class
);
{code}
The test expected the cache to start successfully. However, the previous
implementation did not actually preserve both key-value mappings.
{{CacheConfiguration#setIndexedTypes(...)}} creates a {{QueryEntity}} for each
key-value pair and identifies duplicates by value type. When the second pair
with the same value type was encountered, its {{QueryEntity}} was silently
discarded. As a result, the test effectively verified the same silent metadata
loss that this change is intended to eliminate. Reproducer:
[^DuplicateKeyValueClassesSelfTest.patch]
The test is therefore changed to expect a {{CacheException}} for the
conflicting key types instead of successful cache startup.
This does not affect the supported case where the same key class is used with
different value classes. For example:
{code:java}
setIndexedTypes(
UUID.class, Clazz1.class,
UUID.class, Clazz2.class
);
{code}
still produces two independent query entities because their value types are
different.
> 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: DuplicateKeyValueClassesSelfTest.patch,
> MixedIndexConfigurationTest.patch
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> 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. Reproducer:
> [^MixedIndexConfigurationTest.patch]
> h2. Root cause
> Both methods store query metadata in the same internal {{qryEntities}}
> collection. 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{}}}.
> h2. Merge rules
> 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. Duplicate value class handling
> The existing {{DuplicateKeyValueClassesSelfTest#testDuplicateValueClass}}
> covered the following configuration:
> {code:java}
> setIndexedTypes(
> UUID.class, Clazz1.class,
> String.class, Clazz1.class
> );
> {code}
> The test expected the cache to start successfully. However, the previous
> implementation did not actually preserve both key-value mappings.
> {{CacheConfiguration#setIndexedTypes(...)}} creates a {{QueryEntity}} for
> each key-value pair and identifies duplicates by value type. When the second
> pair with the same value type was encountered, its {{QueryEntity}} was
> silently discarded. As a result, the test effectively verified the same
> silent metadata loss that this change is intended to eliminate. Reproducer:
> [^DuplicateKeyValueClassesSelfTest.patch]
> The test is therefore changed to expect a {{CacheException}} for the
> conflicting key types instead of successful cache startup.
> This does not affect the supported case where the same key class is used with
> different value classes. For example:
> {code:java}
> setIndexedTypes(
> UUID.class, Clazz1.class,
> UUID.class, Clazz2.class
> );
> {code}
> still produces two independent query entities because their value types are
> different.
> h2. IgnitePdsCorruptedIndexTest adjustment
> {\{IgnitePdsCorruptedIndexTest#testCorruption}} previously configured indexed
> types as follows:
> {code}
> .setIndexedTypes(
> Integer.class, IndexedObject.class,
> Long.class, IndexedObject.class
> )
> {code}
> Both pairs use the same value type but different key types. With the previous
> \{{setIndexedTypes(...)}} implementation, the second \{{QueryEntity}} was
> detected as a duplicate by value type and silently discarded. Therefore, the
> effective cache configuration contained only:
> {code}
> Integer -> IndexedObject
> {code}
> The \{{Long -> IndexedObject}} mapping never participated in the actual test
> scenario.
> The purpose of \{{IgnitePdsCorruptedIndexTest}} is to verify index
> consistency/recovery after persistent partition data is deliberately
> corrupted and the cluster is restarted. The test inserts entries with
> \{{Integer}} keys and uses indexes defined on fields of \{{IndexedObject}}.
> The discarded \{{Long -> IndexedObject}} configuration has no effect on this
> scenario.
> The test configuration is therefore changed to:
> {code}
> .setIndexedTypes(Integer.class, IndexedObject.class)
> {code}
> This only removes metadata that was already ignored by the previous
> implementation and does not change the behavior or coverage of the
> corruption/recovery scenario itself.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
