oleg-vlsk commented on code in PR #13583: URL: https://github.com/apache/ignite/pull/13583#discussion_r4056513832
########## modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/CacheConfigurationQueryEntityMergeTest.java: ########## @@ -0,0 +1,1202 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.processors.cache; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import javax.cache.CacheException; +import org.apache.ignite.cache.QueryEntity; +import org.apache.ignite.cache.QueryIndex; +import org.apache.ignite.cache.QueryIndexType; +import org.apache.ignite.cache.query.annotations.QuerySqlField; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.internal.IgniteEx; +import org.apache.ignite.internal.processors.query.QueryUtils; +import org.apache.ignite.spi.systemview.view.SystemView; +import org.apache.ignite.spi.systemview.view.sql.SqlIndexView; +import org.apache.ignite.spi.systemview.view.sql.SqlTableColumnView; +import org.apache.ignite.spi.systemview.view.sql.SqlTableView; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; + +import static org.apache.ignite.internal.processors.query.schema.management.SchemaManager.SQL_TBLS_VIEW; +import static org.apache.ignite.internal.processors.query.schema.management.SchemaManager.SQL_TBL_COLS_VIEW; +import static org.apache.ignite.testframework.GridTestUtils.assertThrows; + +/** Tests for merging QueryEntity metadata in CacheConfiguration. */ +public class CacheConfigurationQueryEntityMergeTest extends GridCommonAbstractTest { + /** */ + private static final String CACHE_NAME = "query-entity-merge-cache"; + + /** */ + private static final String COMPOSITE_IDX = "PERSON_NAME_AGE_IDX"; + + /** */ + private static final String NAME_IDX = "EXPLICIT_NAME_IDX"; + + /** */ + private static final String AGE_IDX = "EXPLICIT_AGE_IDX"; + + /** */ + private static final String NAME_FIELD = "name"; + + /** */ + private static final String AGE_FIELD = "age"; + + /** {@inheritDoc} */ + @Override protected void afterTest() throws Exception { + stopAllGrids(); + + super.afterTest(); + } + + /** Query entities with different value types must not be merged. */ + @Test + public void testDifferentValueTypesAreNotMerged() throws Exception { + IgniteEx node = startGrid(0); + + CacheConfiguration<Integer, Object> ccfg = new CacheConfiguration<>(CACHE_NAME); + + QueryEntity first = new QueryEntity() + .setKeyType(Integer.class.getName()) + .setValueType(Person.class.getName()) + .setFields(fields(NAME_FIELD, String.class)); + + QueryEntity second = new QueryEntity() + .setKeyType(Integer.class.getName()) + .setValueType(AnnotatedPerson.class.getName()) + .setFields(fields(NAME_FIELD, String.class)); + + ccfg.setQueryEntities(Collections.singleton(first)); + ccfg.setQueryEntities(Collections.singleton(second)); + + node.createCache(ccfg); + + Collection<QueryEntity> entities = entities(node); + + assertEquals(2, entities.size()); + + for (Class<?> cls : List.of(Person.class, AnnotatedPerson.class)) + assertTrue(entities.stream().anyMatch(e -> cls.getName().equals(e.getValueType()))); + } + + /** Query entities with the same value type but different key type are a conflict. */ + @Test + public void testConflictingKeyTypesFail() { + CacheConfiguration<Integer, Person> ccfg = new CacheConfiguration<>(CACHE_NAME); + + QueryEntity first = new QueryEntity() + .setKeyType(Integer.class.getName()) + .setValueType(Person.class.getName()) + .setFields(fields(NAME_FIELD, String.class)); + + QueryEntity second = new QueryEntity() + .setKeyType(String.class.getName()) + .setValueType(Person.class.getName()) + .setFields(fields(NAME_FIELD, String.class)); + + ccfg.setQueryEntities(Collections.singleton(first)); + + String msg = String.format("Failed to merge query entities due to conflicting metadata [" + + "cacheName=%s, property=keyType, existingValue=%s, incomingValue=%s]", + CACHE_NAME, Integer.class.getName(), String.class.getName()); + + assertThrows( + log, + () -> ccfg.setQueryEntities(Collections.singleton(second)), + CacheException.class, + msg + ); + } + + /** + * Checks that query entities with conflicting effective key types cannot be merged. + * <p> + * The first entity does not define {@code keyType} explicitly. Instead, its effective key type is derived from + * {@code keyFieldName} and the corresponding field type. The second entity defines a different key type explicitly. + * <p> + * Although {@link QueryEntity#getKeyType()} returns {@code null} for the first entity, + * {@link QueryEntity#findKeyType()} resolves its key type from the field metadata. Therefore, the entities must be + * treated as having conflicting key types. + */ + @Test + public void testConflictingImplicitAndExplicitKeyTypes() { + CacheConfiguration<Integer, Person> ccfg = new CacheConfiguration<>(CACHE_NAME); + + QueryEntity first = new QueryEntity() + .setValueType(Person.class.getName()) + .setFields(fields("id", Integer.class)) + .setKeyFieldName("id"); // keyType is null + + QueryEntity second = new QueryEntity() + .setValueType(Person.class.getName()) + .setKeyType(String.class.getName()); + + ccfg.setQueryEntities(Collections.singleton(first)); + + String msg = String.format("Failed to merge query entities due to conflicting metadata [" + + "cacheName=%s, property=keyType, existingValue=%s, incomingValue=%s]", + CACHE_NAME, Integer.class.getName(), String.class.getName()); + + assertThrows( + log, + () -> ccfg.setQueryEntities(Collections.singleton(second)), + CacheException.class, + msg + ); + } + + /** + * Checks that query entities with matching effective value types can be merged when one value type is defined + * implicitly and the other one explicitly. + * <p> + * The first entity does not define {@code valueType} explicitly. Its effective value type is derived from + * {@code valueFieldName} and the corresponding field type, so {@link QueryEntity#getValueType()} returns + * {@code null}, while {@link QueryEntity#findValueType()} resolves it to the person type. + * <p> + * The second entity defines the same value type explicitly. Since both entities have the same effective value + * type, they must be merged without a conflict. + */ + @Test + public void testMatchingImplicitAndExplicitValueTypesAreMerged() throws Exception { + IgniteEx node = startGrid(0); + + CacheConfiguration<Integer, Person> ccfg = new CacheConfiguration<>(CACHE_NAME); + + QueryEntity first = new QueryEntity() + .setFields(fields("val", Person.class)) + .setValueFieldName("val"); // valueType is null + + QueryEntity second = new QueryEntity().setValueType(Person.class.getName()); + + ccfg.setQueryEntities(Collections.singleton(first)); + ccfg.setQueryEntities(Collections.singleton(second)); + + node.createCache(ccfg); Review Comment: Here 2 entities are merged because their key type is the same (null), so we end up having only one entity [null → Person]. Before my fix there no merging, but because the value type is the same (Person) the second entity was silently discarded. So the result is the same — only 1 entity [null → Person] . The only difference is that if you run this test on master, this assertion ```assertEquals(Person.class.getName(), entity.getValueType());``` will fail, because after my fix all properties of both entities are merged, while now in master the second entity is just ignored. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
