branch-4.1: [fix](fe) block inverted index V1 creation in FE #64522 #67342
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| // Licensed to the Apache Software Foundation (ASF) under one | ||
| // or more contributor license agreements. See the NOTICE file | ||
| // distributed with this work for additional information | ||
| // regarding copyright ownership. The ASF licenses this file | ||
| // to you under the Apache License, Version 2.0 (the | ||
| // "License"); you may not use this file except in compliance | ||
| // with the License. You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, | ||
| // software distributed under the License is distributed on an | ||
| // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| // KIND, either express or implied. See the License for the | ||
| // specific language governing permissions and limitations | ||
| // under the License. | ||
|
|
||
| package org.apache.doris.common; | ||
|
|
||
| import java.lang.reflect.Field; | ||
|
|
||
| public final class InvertedIndexStorageFormatValidator { | ||
| private InvertedIndexStorageFormatValidator() { | ||
| } | ||
|
|
||
| public static void rejectRuntimeV1(String confVal) throws ConfigException { | ||
| String normalizedConfVal = confVal.trim(); | ||
| if ("V1".equalsIgnoreCase(normalizedConfVal)) { | ||
| throw new ConfigException("Inverted index V1 is deprecated and no longer allowed" | ||
| + " for new index creation. Please use inverted index V2."); | ||
| } | ||
| } | ||
|
|
||
| public static void rejectStartupV1(String confVal) throws ConfigException { | ||
| String normalizedConfVal = confVal.trim(); | ||
| if ("V1".equalsIgnoreCase(normalizedConfVal)) { | ||
| throw new ConfigException( | ||
| "inverted_index_storage_format=V1 is no longer supported. " | ||
| + "Please update fe.conf (or fe_custom.conf): set inverted_index_storage_format=V2."); | ||
| } | ||
| } | ||
|
|
||
| public static class RuntimeConfigHandler extends ConfigBase.DefaultConfHandler { | ||
| @Override | ||
| public void handle(Field field, String confVal) throws Exception { | ||
| rejectRuntimeV1(confVal); | ||
| super.handle(field, confVal.trim()); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2816,6 +2816,13 @@ private boolean processAddIndex(CreateIndexClause alterClause, OlapTable olapTab | |
| AnnIndexPropertiesChecker.checkProperties(indexDef.getProperties()); | ||
| } | ||
|
|
||
| if (indexDef.getIndexType() == IndexType.INVERTED | ||
| && olapTable.getInvertedIndexFileStorageFormat() == TInvertedIndexFileStorageFormat.V1) { | ||
|
Contributor
There was a problem hiding this comment. [P1] Treat the legacy DEFAULT sentinel as V1 Tables created before this property existed deserialize its missing FE value as
Member
Author
There was a problem hiding this comment. Thanks — I checked this against master and the same code is there: So the legacy |
||
| throw new DdlException("Inverted index V1 is deprecated and no longer allowed for new index creation." | ||
| + " Upgrading inverted_index_storage_format via ALTER TABLE is not supported;" | ||
| + " recreate the table with inverted_index_storage_format = V2."); | ||
| } | ||
|
|
||
| for (String col : indexDef.getColumns()) { | ||
| Column column = olapTable.getColumn(col); | ||
| if (column != null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Keep a rejected V1 update from resetting persisted configs
For direct
_set_config?inverted_index_storage_format=V1&persist=true, this callback rejects the only value, leavingsetConfigsempty, butSetConfigActionstill callspersistConfig(setConfigs, true). That reset path truncatesfe_custom.conf, so a failed V1 update can erase every unrelated persisted override (and may expose a V1 value fromfe.confat the next startup). Please avoid reset persistence when validation produced errors, or make the request transactional, and add a test that the custom file is unchanged after this rejection.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same upstream-parity situation: master Config.java declares this field with the identical callback = InvertedIndexStorageFormatValidator.RuntimeConfigHandler.class, so the _set_config persist reset-persistence interaction you describe exists on master after #64522 as well. The only local adaptation in this hunk was keeping branch-4.1 bilingual description array (plus re-indentation for checkstyle); the callback wiring is verbatim.
Fixing the SetConfigAction / persistConfig interaction is a genuine improvement but it belongs on master first so both branches behave the same. Leaving this thread open for maintainer visibility.