xtern commented on code in PR #2119:
URL: https://github.com/apache/ignite-3/pull/2119#discussion_r1210535149
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -205,13 +215,84 @@ public CompletableFuture<Void> dropTable(DropTableParams
params) {
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> addColumn(AlterTableAddColumnParams params)
{
- return failedFuture(new UnsupportedOperationException("Not implemented
yet."));
+ if (params.columns().isEmpty()) {
+ return completedFuture(null);
+ }
+
+ return saveUpdate(catalog -> {
+ String schemaName =
Objects.requireNonNullElse(params.schemaName(), CatalogService.PUBLIC);
+
+ SchemaDescriptor schema =
Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " +
schemaName);
+
+ TableDescriptor table = schema.table(params.tableName());
+
+ if (table == null) {
+ throw new TableNotFoundException(schemaName,
params.tableName());
+ }
+
+ List<TableColumnDescriptor> columnDescriptors = new ArrayList<>();
+
+ for (ColumnParams col : params.columns()) {
+ if (table.column(col.name()) != null) {
+ throw new ColumnAlreadyExistsException(col.name());
Review Comment:
- From the user's point of view, I would expect that for a table with an
existing column "column1" the following command "ADD COLUMN IF NOT EXISTS
(column1 int, column2 int)" will silently create 'column2'.
But as far as I understand the current behavior, we just ignore the
exception. It seems to me that it's better to completely forbid 'IF NOT EXISTS'
options for multiple columns if we can't support this (as was in previous PR).
- Why we need `ifColumnNotExists` field in `AlterTableAddColumnParams` if we
don't use it?
- Also, current behavior for ADD COLUMN differs from DROP COLUMN, which
seems strange to me.
I also don't quite understand - why we can't return an empty list here to
prevent catalog version change?
--
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]
