[ 
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:java}
.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:java}
Integer -> IndexedObject
{code}
The {{Long -> IndexedObject}} mapping never participated in the actual test 
scenario.

  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.

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.


> 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:java}
> .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:java}
> Integer -> IndexedObject
> {code}
> The {{Long -> IndexedObject}} mapping never participated in the actual test 
> scenario.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to