ibessonov commented on code in PR #2231:
URL: https://github.com/apache/ignite-3/pull/2231#discussion_r1238087675
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -277,82 +259,62 @@ private Catalog catalogAt(long timestamp) {
return entry.getValue();
}
- /** {@inheritDoc} */
@Override
public CompletableFuture<Void> createTable(CreateTableParams params) {
return saveUpdate(catalog -> {
- String schemaName =
Objects.requireNonNullElse(params.schemaName(), CatalogService.PUBLIC);
-
- CatalogSchemaDescriptor schema =
Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " +
schemaName);
+ CatalogSchemaDescriptor schema = getSchema(catalog,
params.schemaName());
if (schema.table(params.tableName()) != null) {
- throw new TableAlreadyExistsException(schemaName,
params.tableName());
+ throw new TableAlreadyExistsException(schema.name(),
params.tableName());
}
-
params.columns().stream().map(ColumnParams::name).filter(Predicate.not(new
HashSet<>()::add))
- .findAny().ifPresent(columnName -> {
- throw new IgniteInternalException(
-
ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR, "Can't create table with
duplicate columns: "
- +
params.columns().stream().map(ColumnParams::name).collect(Collectors.joining(",
"))
- );
- });
+ validateParams(params);
+
+ CatalogZoneDescriptor zone = getZone(catalog,
Objects.requireNonNullElse(params.zone(), DEFAULT_ZONE_NAME));
Review Comment:
Refactoring that has nothing to do with the fix you provide. `createTable`
is not related to anything described in the original issue, just as any other
"alter" methods. You only changed the part where updates are being converted to
events. I'm all for refactoring that part, it's good, but everything else is
just confusing and unnecessary
--
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]
