korlov42 commented on code in PR #1005:
URL: https://github.com/apache/ignite-3/pull/1005#discussion_r954605301
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/ddl/DropIndexCommand.java:
##########
@@ -21,11 +21,14 @@
* DROP INDEX statement.
*/
public class DropIndexCommand implements DdlCommand {
- /** Idx name. */
+ /** Index name. */
private String indexName;
/** If exist flag. */
- private boolean ifExist;
+ private boolean ifNotExists;
Review Comment:
`drop index if not exists` doesn't make sense to me
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/ddl/CreateIndexCommand.java:
##########
@@ -31,7 +31,7 @@ public class CreateIndexCommand implements DdlCommand {
protected boolean ifTableNotExists;
/** Schema name where this new table will be created. */
- private String commanCurrentSchema;
+ private String commandCurrentSchema;
Review Comment:
does it make sense to rename this to just `schemaName`?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/ddl/DropIndexCommand.java:
##########
@@ -21,11 +21,14 @@
* DROP INDEX statement.
*/
public class DropIndexCommand implements DdlCommand {
- /** Idx name. */
+ /** Index name. */
private String indexName;
/** If exist flag. */
- private boolean ifExist;
+ private boolean ifNotExists;
+
+ /** Schema name where this new table will be created. */
Review Comment:
this is a schema from which the _index_ will be _dropped_
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -225,13 +256,32 @@ public CompletableFuture<Index<?>> createIndexAsync(
*
* @param schemaName A name of the schema the index belong to.
* @param indexName A name of the index to drop.
+ * @param failIfNotExist Flag, which force failure, when {@code trues} if
index doen't not exists.
+ * @return {@code True} if index was removed, {@code false} otherwise.
+ * @throws IndexNotFoundException If index doesn't exist and {@code
failIfNotExist} param was {@code true}.
+ */
+ public boolean dropIndex(
Review Comment:
Do we really need to have sync api in `IndexManager`. I believe the only
reason `TableManager` have such methods is public `IgniteTables`. But
`IndexManager` is internal api
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -334,18 +388,73 @@ private void validateColumns(Iterable<String>
indexedColumns, Set<String> tableC
}
}
- private void onIndexDrop(ConfigurationNotificationEvent<TableIndexView>
ctx) {
- Index<?> index = indexById.remove(ctx.oldValue().id());
- indexByName.remove(index.name(), index);
+ /**
+ * Callback method is called when index configuration changed and an index
was dropped.
+ *
+ * @param evt Index configuration event.
+ * @return A future.
+ */
+ private CompletableFuture<?>
onIndexDrop(ConfigurationNotificationEvent<TableIndexView> evt) {
+ if (!busyLock.enterBusy()) {
+ UUID idxId = ((TableIndexView) evt.newValue()).id();
Review Comment:
I believe for DROP the new value will be NULL.
BTW, cast is redundant here
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -401,17 +512,45 @@ private SortedIndexDescriptor convert(SortedIndexView
indexView) {
);
}
+ /**
+ * Waits for future result and return, or unwraps {@link
CompletionException} to {@link IgniteException} if failed.
+ *
+ * @param future Completable future.
+ * @return Future result.
+ */
+ private <T> T join(CompletableFuture<T> future) {
+ if (!busyLock.enterBusy()) {
+ throw new IgniteException(new NodeStoppingException());
Review Comment:
this constructor is deprecated
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/ddl/CreateIndexCommand.java:
##########
@@ -31,7 +31,7 @@ public class CreateIndexCommand implements DdlCommand {
protected boolean ifTableNotExists;
Review Comment:
The syntax is currently doesn't allow to specify IF TABLE EXISTS thing, so I
would clean up this part and for command too
--
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]
