This is an automated email from the ASF dual-hosted git repository.
mpetrov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 5ce8b57ad9e IGNITE-17041 Fixed SQL query entity aliases validation.
(#10404)
5ce8b57ad9e is described below
commit 5ce8b57ad9e47cac9ee812dbc9d38deb832e886b
Author: Mikhail Petrov <[email protected]>
AuthorDate: Wed Nov 30 22:18:57 2022 +0300
IGNITE-17041 Fixed SQL query entity aliases validation. (#10404)
---
.../java/org/apache/ignite/cache/QueryEntity.java | 57 +++++-
.../cache/ValidationOnNodeJoinUtils.java | 19 ++
.../internal/processors/query/QueryField.java | 24 +++
.../internal/processors/query/QuerySchema.java | 4 +-
.../internal/processors/query/QueryUtils.java | 35 +++-
.../processors/query/QueryEntityAliasesTest.java | 227 +++++++++++++++++++++
.../testsuites/IgniteCacheQuerySelfTestSuite6.java | 4 +-
.../Cache/Query/Linq/CacheLinqTest.Base.cs | 4 +-
.../Cache/Query/Linq/CacheLinqTest.Misc.cs | 2 +-
9 files changed, 354 insertions(+), 22 deletions(-)
diff --git
a/modules/core/src/main/java/org/apache/ignite/cache/QueryEntity.java
b/modules/core/src/main/java/org/apache/ignite/cache/QueryEntity.java
index 41a4b4c37f3..93ba83f53ca 100644
--- a/modules/core/src/main/java/org/apache/ignite/cache/QueryEntity.java
+++ b/modules/core/src/main/java/org/apache/ignite/cache/QueryEntity.java
@@ -262,8 +262,16 @@ public class QueryEntity implements Serializable {
for (Map.Entry<String, String> targetField :
target.getFields().entrySet()) {
String targetFieldName = targetField.getKey();
String targetFieldType = targetField.getValue();
+ String targetFieldAlias = target.getAliases().get(targetFieldName);
if (getFields().containsKey(targetFieldName)) {
+ checkEquals(
+ conflicts,
+ "alias of " + targetFieldName,
+ getAliases().get(targetFieldName),
+ targetFieldAlias
+ );
+
checkEquals(
conflicts,
"fieldType of " + targetFieldName,
@@ -297,23 +305,50 @@ public class QueryEntity implements Serializable {
getFromMap(target.getFieldsScale(), targetFieldName));
}
else {
- Integer precision = getFromMap(target.getFieldsPrecision(),
targetFieldName);
- Integer scale = getFromMap(target.getFieldsScale(),
targetFieldName);
-
- queryFieldsToAdd.add(new QueryField(
- targetFieldName,
- targetFieldType,
- !contains(target.getNotNullFields(), targetFieldName),
- getFromMap(target.getDefaultFieldValues(),
targetFieldName),
- precision == null ? -1 : precision,
- scale == null ? -1 : scale
- ));
+ boolean isAliasConflictsFound =
findAliasConflicts(targetFieldAlias, targetFieldName, conflicts);
+
+ if (!isAliasConflictsFound) {
+ Integer precision =
getFromMap(target.getFieldsPrecision(), targetFieldName);
+ Integer scale = getFromMap(target.getFieldsScale(),
targetFieldName);
+
+ queryFieldsToAdd.add(new QueryField(
+ targetFieldName,
+ targetFieldType,
+ targetFieldAlias,
+ !contains(target.getNotNullFields(), targetFieldName),
+ getFromMap(target.getDefaultFieldValues(),
targetFieldName),
+ precision == null ? -1 : precision,
+ scale == null ? -1 : scale
+ ));
+ }
}
}
return queryFieldsToAdd;
}
+ /**
+ * Checks if received query entity field has the alias which is already
used by a field on the local node.
+ *
+ * @return Whether conflicts were found.
+ */
+ private boolean findAliasConflicts(String targetFieldAlias, String
targetFieldName, StringBuilder conflicts) {
+ for (Map.Entry<String, String> entry : getAliases().entrySet()) {
+ if (Objects.equals(entry.getValue(), targetFieldAlias)) {
+ conflicts.append(String.format(
+ "multiple fields are associated with the same alias:
alias=%s, localField=%s, receivedField=%s\n",
+ targetFieldAlias,
+ entry.getKey(),
+ targetFieldName)
+ );
+
+ return true;
+ }
+ }
+
+ return false;
+ }
+
/**
* @param collection Collection for checking.
* @param elementToCheck Element for checking to containing in collection.
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ValidationOnNodeJoinUtils.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ValidationOnNodeJoinUtils.java
index 1506dcfdf55..cd78df0f170 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ValidationOnNodeJoinUtils.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ValidationOnNodeJoinUtils.java
@@ -94,6 +94,11 @@ public class ValidationOnNodeJoinUtils {
"Failed to join node to the active cluster, configuration conflict for
cache '%s': " +
"schema '%s' from joining node differs to '%s'";
+ /** Template of message of conflicts of {@link
CacheConfiguration#isSqlEscapeAll()} flag. */
+ private static final String SQL_ESCAPE_ALL_CONFLICT_MESSAGE =
+ "Failed to join node to the cluster, configuration conflict for cache
'%s': " +
+ "\"isSqlEscapeAll\" configuration property is different [local=%s,
remote=%s]";
+
/** Template of message of conflicts during configuration merge. */
private static final String MERGE_OF_CONFIG_CONFLICTS_MESSAGE =
"Conflicts during configuration merge for cache '%s' : \n%s";
@@ -188,6 +193,20 @@ public class ValidationOnNodeJoinUtils {
locSchema));
}
+ if (!cacheInfo.cacheData().queryEntities().isEmpty() &&
!locDesc.cacheConfiguration().getQueryEntities().isEmpty()) {
+ boolean locSqlEscapeAll =
locDesc.cacheConfiguration().isSqlEscapeAll();
+ boolean rmtSqlEscapeAll =
cacheInfo.cacheData().config().isSqlEscapeAll();
+
+ if (locSqlEscapeAll != rmtSqlEscapeAll) {
+ errorMsg.append(String.format(
+ SQL_ESCAPE_ALL_CONFLICT_MESSAGE,
+ locDesc.cacheName(),
+ locSqlEscapeAll,
+ rmtSqlEscapeAll)
+ );
+ }
+ }
+
QuerySchemaPatch schemaPatch =
locDesc.makeSchemaPatch(joinedQryEntities);
if (schemaPatch.hasConflicts() || (isGridActive &&
!schemaPatch.isEmpty())) {
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryField.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryField.java
index 9209364af58..fb490529e24 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryField.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryField.java
@@ -30,6 +30,9 @@ public class QueryField implements Serializable {
/** Field name. */
private final String name;
+ /** Alias. */
+ private final String alias;
+
/** Class name for this field's values. */
private final String typeName;
@@ -73,8 +76,22 @@ public class QueryField implements Serializable {
* @param scale Scale.
*/
public QueryField(String name, String typeName, boolean nullable, Object
dfltValue, int precision, int scale) {
+ this(name, typeName, null, nullable, dfltValue, precision, scale);
+ }
+
+ /**
+ * @param name Field name.
+ * @param typeName Class name for this field's values.
+ * @param alias Alias.
+ * @param nullable Nullable flag.
+ * @param dfltValue Default value.
+ * @param precision Precision.
+ * @param scale Scale.
+ */
+ public QueryField(String name, String typeName, String alias, boolean
nullable, Object dfltValue, int precision, int scale) {
this.name = name;
this.typeName = typeName;
+ this.alias = alias;
this.nullable = nullable;
this.dfltValue = dfltValue;
this.precision = precision;
@@ -88,6 +105,13 @@ public class QueryField implements Serializable {
return name;
}
+ /**
+ * @return Field alias.
+ */
+ public String alias() {
+ return alias != null ? alias : name;
+ }
+
/**
* @return Class name for this field's values.
*/
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QuerySchema.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QuerySchema.java
index 5fac11c5246..3e549feac8a 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QuerySchema.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QuerySchema.java
@@ -279,7 +279,7 @@ public class QuerySchema implements Serializable {
QueryEntity target =
((List<QueryEntity>)entities).get(targetIdx);
for (QueryField field : op0.columns()) {
- target.getFields().put(field.name(), field.typeName());
+ target.addQueryField(field.name(), field.typeName(),
field.alias());
if (!field.isNullable()) {
if (!(target instanceof QueryEntityEx)) {
@@ -326,7 +326,7 @@ public class QuerySchema implements Serializable {
QueryEntity entity =
((List<QueryEntity>)entities).get(targetIdx);
for (String field : op0.columns()) {
- boolean rmv = QueryUtils.removeField(entity, field);
+ boolean rmv = QueryUtils.removeFieldAndAlias(entity,
field);
assert rmv || op0.ifExists() : "Invalid operation state
[removed=" + rmv
+ ", ifExists=" + op0.ifExists() + ']';
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryUtils.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryUtils.java
index ecf9eaf28da..c0127c2994a 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryUtils.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/QueryUtils.java
@@ -1341,6 +1341,8 @@ public class QueryUtils {
", valFieldName=" + valFieldName + "]");
}
+ validateAliases(entity);
+
Collection<QueryIndex> idxs = entity.getIndexes();
if (!F.isEmpty(idxs)) {
@@ -1400,6 +1402,21 @@ public class QueryUtils {
}
}
+ /**
+ * @param entity Query entity which aliases should be validated.
+ * @throws IgniteException If validation failed.
+ */
+ private static void validateAliases(QueryEntity entity) {
+ Set<String> aliases = new HashSet<>();
+
+ for (String alias : entity.getAliases().values()) {
+ if (!aliases.add(alias)) {
+ throw new IgniteException(
+ "Multiple query fields are associated with the same alias
[alias=" + alias + "]");
+ }
+ }
+ }
+
/**
* Construct cache name for table.
*
@@ -1704,14 +1721,22 @@ public class QueryUtils {
}
/**
- * Remove field by alias.
+ * Remove field and corresponding alias by the alias name.
*
* @param entity Query entity.
- * @param alias Filed's alias.
- * @return {@code true} if the field is removed. Otherwise returns {@code
false}.
+ * @param alias Name of the field alias.
+ * @return {@code true} if the field and corresponding alias is removed.
Otherwise, returns {@code false}.
*/
- public static boolean removeField(QueryEntity entity, String alias) {
- return entity.getFields().remove(fieldNameByAlias(entity, alias)) !=
null;
+ public static boolean removeFieldAndAlias(QueryEntity entity, String
alias) {
+ String fieldName = fieldNameByAlias(entity, alias);
+
+ if (entity.getFields().remove(fieldName) != null) {
+ entity.getAliases().remove(fieldName);
+
+ return true;
+ }
+
+ return false;
}
/**
diff --git
a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/QueryEntityAliasesTest.java
b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/QueryEntityAliasesTest.java
new file mode 100644
index 00000000000..882ee542c92
--- /dev/null
+++
b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/QueryEntityAliasesTest.java
@@ -0,0 +1,227 @@
+/*
+ * 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.query;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.SqlFieldsQuery;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.spi.IgniteSpiException;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.cluster.ClusterState.ACTIVE;
+import static org.apache.ignite.cluster.ClusterState.INACTIVE;
+import static
org.apache.ignite.testframework.GridTestUtils.assertThrowsAnyCause;
+
+/** */
+public class QueryEntityAliasesTest extends GridCommonAbstractTest {
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String
igniteInstanceName) throws Exception {
+ return
getConfiguration(getTestIgniteInstanceIndex(igniteInstanceName),
getTestQueryEntity());
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ super.afterTest();
+
+ stopAllGrids();
+ }
+
+ /** */
+ @Test
+ public void testSqlEscapeFlagMismatch() throws Exception {
+ startGrid(0);
+
+ assertThrowsAnyCause(
+ log,
+ () -> {
+ startGrid(getConfiguration(1,
getTestQueryEntity().addQueryField("salary", Integer.class.getName(), null),
true));
+
+ return null;
+ },
+ IgniteSpiException.class,
+ "Failed to join node to the cluster, configuration conflict for
cache 'default': \"isSqlEscapeAll\"" +
+ " configuration property is different [local=false,
remote=true]"
+ );
+
+ checkTableColumns("_KEY", "_VAL", "ID", "AGE");
+ }
+
+ /** */
+ @Test
+ public void testQueryEntityAliasesValidation() throws Exception {
+ startGrid(0);
+
+ assertThrowsAnyCause(
+ log,
+ () -> {
+ startGrid(getConfiguration(1,
getTestQueryEntity().addQueryField("salary", Integer.class.getName(), "AGE")));
+
+ return null;
+ },
+ IgniteException.class,
+ "Multiple query fields are associated with the same alias
[alias=AGE]"
+ );
+
+ assertThrowsAnyCause(
+ log,
+ () -> {
+ grid(0).createCache(new CacheConfiguration<>("test-cache")
+
.setQueryEntities(Collections.singletonList(getTestQueryEntity()
+ .addQueryField("salary", Integer.class.getName(),
"AGE"))));
+
+ return null;
+ },
+ IgniteException.class,
+ "Multiple query fields are associated with the same alias
[alias=AGE]"
+ );
+ }
+
+ /** */
+ @Test
+ public void testQueryEntityEntityMergeAliasesConflicts() throws Exception {
+ startGrid(0);
+
+ assertThrowsAnyCause(
+ log,
+ () -> {
+ startGrid(getConfiguration(1, new QueryEntity(String.class,
Person.class)
+ .setTableName("PERSON")
+ .addQueryField("age", Integer.class.getName(), "A")));
+
+ return null;
+ },
+ IgniteSpiException.class,
+ "alias of age is different: local=AGE, received=A"
+ );
+
+ assertThrowsAnyCause(
+ log,
+ () -> {
+ startGrid(getConfiguration(1, new QueryEntity(String.class,
Person.class)
+ .setTableName("PERSON")
+ .addQueryField("salary", Integer.class.getName(), "AGE")));
+
+ return null;
+ },
+ IgniteSpiException.class,
+ "multiple fields are associated with the same alias: alias=AGE,
localField=age, receivedField=salary"
+ );
+
+ checkTableColumns("_KEY", "_VAL", "ID", "AGE");
+
+ grid(0).cache(DEFAULT_CACHE_NAME).query(new SqlFieldsQuery("ALTER
TABLE PERSON ADD salary INTEGER")).getAll();
+
+ assertThrowsAnyCause(
+ log,
+ () -> {
+ startGrid(getConfiguration(1, new QueryEntity(String.class,
Person.class)
+ .setTableName("PERSON")
+ .addQueryField("SALARY", Integer.class.getName(), "S")));
+
+ return null;
+ },
+ IgniteSpiException.class,
+ "alias of SALARY is different: local=SALARY, received=S"
+ );
+
+ assertThrowsAnyCause(
+ log,
+ () -> {
+ startGrid(getConfiguration(1, new QueryEntity(String.class,
Person.class)
+ .setTableName("PERSON")
+ .addQueryField("salary", Integer.class.getName(), null)));
+
+ return null;
+ },
+ IgniteSpiException.class,
+ "multiple fields are associated with the same alias: alias=SALARY,
localField=SALARY, receivedField=salary"
+ );
+
+ checkTableColumns("_KEY", "_VAL", "ID", "AGE", "SALARY");
+ }
+
+
+ /** */
+ @Test
+ public void testTableColumnNamesAfterSuccessfulQueryEntityMerge() throws
Exception {
+ startGrid(0);
+
+ grid(0).cluster().state(INACTIVE);
+
+ startGrid(getConfiguration(1,
getTestQueryEntity().addQueryField("salary", Integer.class.getName(), null)));
+
+ grid(0).cluster().state(ACTIVE);
+
+ checkTableColumns("_KEY", "_VAL", "AGE", "SALARY", "ID");
+ }
+
+ /** */
+ private void checkTableColumns(String... expCols) {
+ List<?> cols = grid(0).cache(DEFAULT_CACHE_NAME).query(new
SqlFieldsQuery("select * from \"SYS\".TABLE_COLUMNS"))
+ .getAll()
+ .stream()
+ .map(val -> (String)val.get(0))
+ .collect(Collectors.toList());
+
+ List<String> exp = Arrays.asList(expCols);
+
+ assertEquals(exp.size(), cols.size());
+ assertTrue(cols.containsAll(exp));
+ }
+
+ /** */
+ private QueryEntity getTestQueryEntity() {
+ return new QueryEntity(String.class, Person.class)
+ .setTableName("PERSON")
+ .addQueryField("id", Integer.class.getName(), "ID")
+ .addQueryField("age", Integer.class.getName(), "AGE");
+ }
+
+ /** */
+ private IgniteConfiguration getConfiguration(int nodeIdx, QueryEntity
qryEntity) throws Exception {
+ return getConfiguration(nodeIdx, qryEntity, false);
+ }
+
+ /** */
+ private IgniteConfiguration getConfiguration(int nodeIdx, QueryEntity
qryEntity, boolean isSqlEscapeAll) throws Exception {
+ CacheConfiguration<?, ?> ccfg = new
CacheConfiguration<>(DEFAULT_CACHE_NAME)
+ .setSqlEscapeAll(isSqlEscapeAll)
+ .setQueryEntities(Collections.singletonList(qryEntity));
+
+ return
super.getConfiguration(getTestIgniteInstanceName(nodeIdx)).setCacheConfiguration(ccfg);
+ }
+
+ /** */
+ class Person {
+ /** */
+ private int id;
+
+ /** */
+ private int age;
+
+ /** */
+ private int salary;
+ }
+}
diff --git
a/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteCacheQuerySelfTestSuite6.java
b/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteCacheQuerySelfTestSuite6.java
index 24621944bf6..77edde1f7d6 100644
---
a/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteCacheQuerySelfTestSuite6.java
+++
b/modules/indexing/src/test/java/org/apache/ignite/testsuites/IgniteCacheQuerySelfTestSuite6.java
@@ -42,6 +42,7 @@ import
org.apache.ignite.internal.processors.performancestatistics.PerformanceSt
import
org.apache.ignite.internal.processors.query.IgniteInsertNullableDuplicatesSqlTest;
import
org.apache.ignite.internal.processors.query.IgniteSqlSinglePartitionMultiParallelismTest;
import
org.apache.ignite.internal.processors.query.MemLeakOnSqlWithClientReconnectTest;
+import org.apache.ignite.internal.processors.query.QueryEntityAliasesTest;
import org.junit.runner.RunWith;
import org.junit.runners.Suite;
@@ -75,7 +76,8 @@ import org.junit.runners.Suite;
PerformanceStatisticsQueryTest.class,
CacheContinuousQueryFilterDeploymentFailedTest.class,
CacheContinuousQueryDeploymentToClientTest.class,
- IgniteSqlSinglePartitionMultiParallelismTest.class
+ IgniteSqlSinglePartitionMultiParallelismTest.class,
+ QueryEntityAliasesTest.class
})
public class IgniteCacheQuerySelfTestSuite6 {
}
diff --git
a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Base.cs
b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Base.cs
index 8bcf4c053bb..209184505b5 100644
---
a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Base.cs
+++
b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Base.cs
@@ -91,7 +91,7 @@ namespace Apache.Ignite.Core.Tests.Cache.Query.Linq
{
cache.Put(i, new Person(i, string.Format(" Person_{0} ", i))
{
- Address = new Address {Zip = i, Street = "Street " + i,
AliasTest = i},
+ Address = new Address {Zip = i, Street = "Street " + i,
AddressAliasTest = i},
OrganizationId = i%2 + 1000,
Birthday = StartDateTime.AddYears(i),
AliasTest = -i
@@ -414,7 +414,7 @@ namespace Apache.Ignite.Core.Tests.Cache.Query.Linq
{
[QuerySqlField] public int Zip { get; set; }
[QuerySqlField] public string Street { get; set; }
- [QuerySqlField] public int AliasTest { get; set; }
+ [QuerySqlField] public int AddressAliasTest { get; set; }
}
public class Organization
diff --git
a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Misc.cs
b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Misc.cs
index 9c8370bead8..df5e893f6db 100644
---
a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Misc.cs
+++
b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Linq/CacheLinqTest.Misc.cs
@@ -201,7 +201,7 @@ namespace Apache.Ignite.Core.Tests.Cache.Query.Linq
var cache = GetPersonCache().AsCacheQueryable();
var res = cache.Where(x => x.Key == 1)
- .Select(x => new { X = x.Value.AliasTest, Y =
x.Value.Address.AliasTest })
+ .Select(x => new { X = x.Value.AliasTest, Y =
x.Value.Address.AddressAliasTest })
.Single();
Assert.AreEqual(new { X = -1, Y = 1 }, res);
