Vladsz83 commented on code in PR #12453:
URL: https://github.com/apache/ignite/pull/12453#discussion_r2481961477
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java:
##########
@@ -2350,7 +2365,7 @@ private void registerCache0(
boolean isSql
) throws IgniteCheckedException {
synchronized (stateMux) {
- if (moduleEnabled()) {
+ if (moduleEnabled() && !cacheNamesToSchema.containsKey(cacheName))
{
Review Comment:
An optimization? How it worked before?
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java:
##########
@@ -392,15 +393,84 @@ public void createTableCustomSchema() {
*/
@Test
public void createTableOnExistingCache() {
- IgniteCache<Object, Object> cache =
client.getOrCreateCache("my_cache");
+ // Cache without SQL configuration.
+ IgniteCache<Object, Object> cache =
client.getOrCreateCache("my_cache0");
- sql("create table my_schema.my_table (f1 int, f2 varchar) with
cache_name=\"my_cache\"");
+ // DDL with explicit schema.
+ sql("create table my_schema.my_table (f1 int, f2 varchar) with
cache_name=\"my_cache0\"");
- sql("insert into my_schema.my_table(f1, f2) values (1, '1'),(2, '2')");
+ checkSize(cache, "my_schema");
+
+ // Cache without SQL configuration.
+ cache = client.getOrCreateCache("my_cache1");
+
+ // DDL with implicit PUBLIC schema.
+ sql("create table my_table (f1 int, f2 varchar) with
cache_name=\"my_cache1\"");
+
+ checkSize(cache, "public");
+
+ // Cache with defined schema.
+ cache = client.getOrCreateCache(new
CacheConfiguration<>("my_cache2").setSqlSchema("my_schema2"));
+
+ // DDL with explicit correct schema.
+ sql("create table my_schema2.my_table (f1 int, f2 varchar) with
cache_name=\"my_cache2\"");
+
+ checkSize(cache, "my_schema2");
+
+ // Cache with defined schema.
+ cache = client.getOrCreateCache(new
CacheConfiguration<>("my_cache3").setSqlSchema("my_schema3"));
+
+ // DDL with explicit wrong schema.
+ assertThrows("create table my_schema.my_table2 (f1 int, f2 varchar)
with cache_name=\"my_cache3\"",
+ IgniteSQLException.class, "Invalid schema: MY_SCHEMA");
+
+ // DDL with explicit wrong schema.
+ assertThrows("create table public.my_table2 (f1 int, f2 varchar) with
cache_name=\"my_cache3\"",
+ IgniteSQLException.class, "Invalid schema: PUBLIC");
+
+ // DDL with implicit wrong schema.
+ assertThrows("create table my_table2 (f1 int, f2 varchar) with
cache_name=\"my_cache3\"",
+ IgniteSQLException.class, "Invalid schema: PUBLIC");
+
+ // DDL woth implicit cache schema.
Review Comment:
woth -> with
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java:
##########
@@ -392,15 +393,84 @@ public void createTableCustomSchema() {
*/
@Test
public void createTableOnExistingCache() {
- IgniteCache<Object, Object> cache =
client.getOrCreateCache("my_cache");
+ // Cache without SQL configuration.
+ IgniteCache<Object, Object> cache =
client.getOrCreateCache("my_cache0");
- sql("create table my_schema.my_table (f1 int, f2 varchar) with
cache_name=\"my_cache\"");
+ // DDL with explicit schema.
+ sql("create table my_schema.my_table (f1 int, f2 varchar) with
cache_name=\"my_cache0\"");
- sql("insert into my_schema.my_table(f1, f2) values (1, '1'),(2, '2')");
+ checkSize(cache, "my_schema");
+
+ // Cache without SQL configuration.
+ cache = client.getOrCreateCache("my_cache1");
+
+ // DDL with implicit PUBLIC schema.
+ sql("create table my_table (f1 int, f2 varchar) with
cache_name=\"my_cache1\"");
+
+ checkSize(cache, "public");
+
+ // Cache with defined schema.
+ cache = client.getOrCreateCache(new
CacheConfiguration<>("my_cache2").setSqlSchema("my_schema2"));
+
+ // DDL with explicit correct schema.
+ sql("create table my_schema2.my_table (f1 int, f2 varchar) with
cache_name=\"my_cache2\"");
+
+ checkSize(cache, "my_schema2");
+
+ // Cache with defined schema.
+ cache = client.getOrCreateCache(new
CacheConfiguration<>("my_cache3").setSqlSchema("my_schema3"));
+
+ // DDL with explicit wrong schema.
+ assertThrows("create table my_schema.my_table2 (f1 int, f2 varchar)
with cache_name=\"my_cache3\"",
+ IgniteSQLException.class, "Invalid schema: MY_SCHEMA");
+
+ // DDL with explicit wrong schema.
+ assertThrows("create table public.my_table2 (f1 int, f2 varchar) with
cache_name=\"my_cache3\"",
+ IgniteSQLException.class, "Invalid schema: PUBLIC");
+
+ // DDL with implicit wrong schema.
+ assertThrows("create table my_table2 (f1 int, f2 varchar) with
cache_name=\"my_cache3\"",
+ IgniteSQLException.class, "Invalid schema: PUBLIC");
+
+ // DDL woth implicit cache schema.
+ cache.query(new SqlFieldsQuery("create table my_table (f1 int, f2
varchar) with cache_name=\"my_cache3\""));
+
+ checkSize(cache, "my_schema3");
+
+ // Cache with defined SQL functions, schema is defined by cache name.
+ cache = client.getOrCreateCache(new
CacheConfiguration<>("my_cache4").setSqlFunctionClasses(getClass()));
+
+ // DDL with explicit wrong schema.
+ assertThrows("create table public.my_table2 (f1 int, f2 varchar) with
cache_name=\"my_cache4\"",
+ IgniteSQLException.class, "Invalid schema: PUBLIC");
+
+ // DDL with explicit correct schema.
+ sql("create table \"my_cache4\".my_table (f1 int, f2 varchar) with
cache_name=\"my_cache4\"");
+
+ checkSize(cache, "\"my_cache4\"");
+
+ // Cache with defined query entities.
+ client.getOrCreateCache(new CacheConfiguration<>("my_cache5")
+ .setQueryEntities(Collections.singleton(new
QueryEntity(Integer.class, Integer.class))));
+
+ assertThrows("create table \"my_cache5\".my_table (f1 int, f2 varchar)
with cache_name=\"my_cache5\"",
+ IgniteSQLException.class, "Cache is already indexed");
+
+ // Cache with indexed types.
+ client.getOrCreateCache(new CacheConfiguration<>("my_cache6")
+ .setIndexedTypes(Integer.class, Integer.class));
+
+ assertThrows("create table \"my_cache6\".my_table (f1 int, f2 varchar)
with cache_name=\"my_cache6\"",
+ IgniteSQLException.class, "Cache is already indexed");
+ }
+
+ /** */
+ private void checkSize(IgniteCache<?, ?> cache, String schema) {
Review Comment:
Minor. It can check size only of an empty cache. Let's add a comment or
rename to smth. like 'checkSizeOnEmpty' or even 'insertAndCheckSize'
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java:
##########
@@ -1696,8 +1696,21 @@ else if (op instanceof
SchemaAlterTableDropColumnOperation) {
}
}
else if (op instanceof SchemaAddQueryEntityOperation) {
- if (cacheNames.contains(op.cacheName()))
- err = new
SchemaOperationException(SchemaOperationException.CODE_CACHE_ALREADY_INDEXED,
op.cacheName());
+ String cacheSchema = cacheNamesToSchema.get(cacheName);
+
+ if (cacheSchema != null) {
+ if (!Objects.equals(cacheSchema, op.schemaName()))
+ err = new
SchemaOperationException(SchemaOperationException.CODE_INVALID_SCHEMA,
op.schemaName());
+ else {
+ for (QueryTypeIdKey t : types.keySet()) {
Review Comment:
isn't it enough just `err = new
SchemaOperationException(SchemaOperationException.CODE_CACHE_ALREADY_INDEXED,
cacheName);` Why we traverse all the types? As I can see, it fails on the same
cache name. Do we have test where it can fail at other cache name?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java:
##########
@@ -2173,15 +2190,13 @@ else if (op instanceof
SchemaAlterTableDropColumnOperation) {
else if (op instanceof SchemaAddQueryEntityOperation) {
SchemaAddQueryEntityOperation op0 =
(SchemaAddQueryEntityOperation)op;
- if (!cacheNames.contains(op0.cacheName())) {
Review Comment:
Why no any analogue?
--
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]
