[
https://issues.apache.org/jira/browse/IGNITE-28907?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Oleg Valuyskiy updated IGNITE-28907:
------------------------------------
Description:
CacheConfiguration currently handles repeated and mixed calls to
setIndexedTypes() and setQueryEntities() inconsistently:
* Mixing the two APIs can silently discard query entity metadata, including
explicitly configured indexes.
* Repeated setQueryEntities() calls append entities but silently skip those
whose value type is already present.
* setIndexedTypes() contains a guard intended to reject repeated calls rather
than replace the previous configuration.
It is suggested to introduce an explicit configuration contract:
* - Disallow mixing setIndexedTypes() and setQueryEntities() on the same
configuration instance. Throw CacheException regardless of the call order.
- Repeated setIndexedTypes() calls replace the previously configured indexed
types and query entities.
- Repeated setQueryEntities() calls replace the entire query entities
collection.
- Passing an empty array or null to setIndexedTypes() clears indexed types and
query entities. Passing an empty collection to setQueryEntities() clears query
entities.
- Empty calls still establish the configuration API. clearQueryEntities() does
not reset that choice.
- Affinity key configurations derived by setIndexedTypes() replace existing
mappings for the same types while preserving mappings for other types. Empty
calls leave key configurations unchanged.
Internal query entity updates during cache initialization and schema
synchronization must use a separate replacement mechanism that does not change
the configuration API marker and preserves QueryEntityEx metadata.
This intentionally changes the public configuration contract. Applications that
mix the two APIs must use a single API, and applications that rely on additive
setQueryEntities() calls must provide the complete collection in the final call.
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: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.
h2. IgnitePdsIndexingDefragmentationTest adjustment
{{IgnitePdsIndexingDefragmentationTest}} previously configured both caches with
two indexed type pairs sharing the same value type:
{code:java}
.setIndexedTypes(
IgniteCacheUpdateSqlQuerySelfTest.AllTypes.class, byte[].class,
Integer.class, byte[].class
)
{code}
However, the previous {{setIndexedTypes(...)}} implementation identified
duplicate {{QueryEntity}} instances by value type and silently discarded the
second one. As a result, despite two key-value pairs being specified, the
effective cache configuration contained only one {{{}QueryEntity{}}}:
{code:java}
AllTypes -> byte[]
{code}
The {{Integer -> byte[]}} configuration was never actually preserved.
Reproducer: [^IgnitePdsIndexingDefragmentationTest.patch]
This is especially relevant because the test class contains separate scenarios
intended to exercise indexing with a simple {{Integer}} key and with a complex
{{AllTypes}} key. Previously, those scenarios used different key objects when
populating the cache, but the configured {{QueryEntity}} key type was not
verified. Therefore, the test could pass even though both scenarios effectively
used SQL metadata configured for {{{}AllTypes{}}}.
The test setup is changed so that each scenario configures exactly the key type
that it actually uses:
{code:java}
testIndexingWithIntegerKey:
Integer -> byte[]
testIndexingWithComplexKey:
AllTypes -> byte[]
{code}
Additional assertions verify both the configured {{QueryEntity}} key type and
the actual class of the keys stored in the cache. This change does not alter
the defragmentation scenario itself.
> Disallow mixing setIndexedTypes and setQueryEntities and use last-call-wins
> semantics
> -------------------------------------------------------------------------------------
>
> 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: MixedIndexConfigurationTest.patch
>
> Time Spent: 3h
> Remaining Estimate: 0h
>
> CacheConfiguration currently handles repeated and mixed calls to
> setIndexedTypes() and setQueryEntities() inconsistently:
> * Mixing the two APIs can silently discard query entity metadata, including
> explicitly configured indexes.
> * Repeated setQueryEntities() calls append entities but silently skip those
> whose value type is already present.
> * setIndexedTypes() contains a guard intended to reject repeated calls
> rather than replace the previous configuration.
> It is suggested to introduce an explicit configuration contract:
> * - Disallow mixing setIndexedTypes() and setQueryEntities() on the same
> configuration instance. Throw CacheException regardless of the call order.
> - Repeated setIndexedTypes() calls replace the previously configured indexed
> types and query entities.
> - Repeated setQueryEntities() calls replace the entire query entities
> collection.
> - Passing an empty array or null to setIndexedTypes() clears indexed types
> and query entities. Passing an empty collection to setQueryEntities() clears
> query entities.
> - Empty calls still establish the configuration API. clearQueryEntities()
> does not reset that choice.
> - Affinity key configurations derived by setIndexedTypes() replace existing
> mappings for the same types while preserving mappings for other types. Empty
> calls leave key configurations unchanged.
> Internal query entity updates during cache initialization and schema
> synchronization must use a separate replacement mechanism that does not
> change the configuration API marker and preserves QueryEntityEx metadata.
> This intentionally changes the public configuration contract. Applications
> that mix the two APIs must use a single API, and applications that rely on
> additive setQueryEntities() calls must provide the complete collection in the
> final call.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
