korlov42 commented on code in PR #3590:
URL: https://github.com/apache/ignite-3/pull/3590#discussion_r1565328032
##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/DropZoneCommandValidationTest.java:
##########
@@ -47,15 +46,6 @@ void zoneNameMustNotBeNullOrBlank(String zone) {
);
}
- @Test
- void rejectToDropDefaultZone() {
Review Comment:
I think, this test case is still valid
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java:
##########
@@ -648,16 +649,21 @@ private SystemView<?> createSystemViewColumnsView() {
}
private SystemView<?> createZonesView() {
- return SystemViews.<CatalogZoneDescriptor>clusterViewBuilder()
+ return SystemViews.<ZoneWithDefaultMarker>clusterViewBuilder()
.name("ZONES")
- .addColumn("NAME", STRING, CatalogZoneDescriptor::name)
- .addColumn("PARTITIONS", INT32,
CatalogZoneDescriptor::partitions)
- .addColumn("REPLICAS", INT32, CatalogZoneDescriptor::replicas)
- .addColumn("DATA_NODES_AUTO_ADJUST_SCALE_UP", INT32,
CatalogZoneDescriptor::dataNodesAutoAdjustScaleUp)
- .addColumn("DATA_NODES_AUTO_ADJUST_SCALE_DOWN", INT32,
CatalogZoneDescriptor::dataNodesAutoAdjustScaleDown)
- .addColumn("DATA_NODES_FILTER", STRING,
CatalogZoneDescriptor::filter)
- .addColumn("IS_DEFAULT_ZONE", BOOLEAN, isDefaultZone())
- .dataProvider(SubscriptionUtils.fromIterable(() ->
catalogAt(clockService.nowLong()).zones().iterator()))
+ .addColumn("NAME", STRING, wrapper -> wrapper.zone.name())
+ .addColumn("PARTITIONS", INT32, wrapper ->
wrapper.zone.partitions())
+ .addColumn("REPLICAS", INT32, wrapper ->
wrapper.zone.replicas())
+ .addColumn("DATA_NODES_AUTO_ADJUST_SCALE_UP", INT32, wrapper
-> wrapper.zone.dataNodesAutoAdjustScaleUp())
+ .addColumn("DATA_NODES_AUTO_ADJUST_SCALE_DOWN", INT32, wrapper
-> wrapper.zone.dataNodesAutoAdjustScaleDown())
+ .addColumn("DATA_NODES_FILTER", STRING, wrapper ->
wrapper.zone.filter())
+ .addColumn("IS_DEFAULT_ZONE", BOOLEAN, wrapper ->
wrapper.isDefault)
Review Comment:
it would be nice of you to specify type of columns explicitly according to
System view registration guidelines. Since this is totally unrelated to
original issue, feel free to ignore this comment
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateTableCommand.java:
##########
@@ -110,7 +109,11 @@ public List<UpdateEntry> get(Catalog catalog) {
ensureNoTableIndexOrSysViewExistsWithGivenName(schema, tableName);
- CatalogZoneDescriptor zone = zoneOrThrow(catalog, zoneName);
+ CatalogZoneDescriptor zone = zoneName == null
+ ? catalog.zone(catalog.defaultZone().id())
Review Comment:
why don't just get zone returned by `catalog.defaultZone()`?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/DistributionZoneSqlDdlParserTest.java:
##########
@@ -179,6 +179,33 @@ public void alterZoneIfExistsRenameTo() {
expectUnparsed(alterZone, expectedStmt);
}
+ /**
+ * Parsing ALTER ZONE SET DEFAULT statement.
+ */
+ @Test
+ public void alterZoneSetDefault() {
Review Comment:
let's add test for negative scenario where within a single command we change
any attribute of the zone and set it as default
--
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]
