Copilot commented on code in PR #52251:
URL: https://github.com/apache/doris/pull/52251#discussion_r2166419767
##########
fe/fe-core/src/test/java/org/apache/doris/alter/IndexChangeJobTest.java:
##########
@@ -685,5 +710,15 @@ public void testCancelNgramBfBuildIndex() throws
UserException {
schemaChangeHandler.runAfterCatalogReady();
Assert.assertEquals(AlterJobV2.JobState.RUNNING, jobV2.getJobState());
Assert.assertEquals(1, jobV2.schemaChangeBatchTask.getTaskNum());
+
+ TableNameInfo tableNameInfo = new TableNameInfo(db.getName(),
table.getName());
+ CancelAlterTableCommand cancelAlterTableCommand = new
CancelAlterTableCommand(
+ tableNameInfo,
+ CancelAlterTableCommand.AlterType.COLUMN,
Review Comment:
The test uses `AlterType.COLUMN` when creating `CancelAlterTableCommand`,
but this scenario is cancelling an index change. It should use the
`AlterType.INDEX` enum to ensure the correct alter-job type is cancelled.
```suggestion
CancelAlterTableCommand.AlterType.INDEX,
```
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -2092,9 +2087,21 @@ public int getAsInt() {
}
lightSchemaChange = false;
+ // Check if the index supports light index change and
session variable is enabled
+ boolean enableLightAddIndex = true;
+ try {
+ ConnectContext context = ConnectContext.get();
+ if (context != null && context.getSessionVariable() !=
null) {
+ enableLightAddIndex =
context.getSessionVariable().isEnableLightAddIndex();
+ }
+ } catch (Exception e) {
+ LOG.warn("Failed to get session variable
enable_light_index_change, "
Review Comment:
The warning references `enable_light_index_change`, but the actual variable
is named `enable_light_add_index`. Update the message to avoid confusion.
```suggestion
LOG.warn("Failed to get session variable
enable_light_add_index, "
```
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -2165,17 +2172,12 @@ public int getAsInt() {
if (alterIndexes.isEmpty()) {
throw new DdlException("Altered index is empty. please
check your alter stmt.");
}
- IndexDef.IndexType indexType =
alterIndexes.get(0).getIndexType();
if (Config.enable_light_index_change) {
Review Comment:
`Config.enable_light_index_change` is not a defined configuration flag. It
should reference the new session variable (`enableLightAddIndex`) or
corresponding constant.
```suggestion
if
(ConnectContext.get().getSessionVariable().enableLightAddIndex) {
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
