github-actions[bot] commented on code in PR #69761:
URL: https://github.com/apache/doris/pull/69761#discussion_r4421823997
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java:
##########
@@ -562,6 +583,77 @@ public Map<String, String> getProperties() {
return properties;
}
+ public boolean isVariantRootIndex() {
+ return indexType == IndexType.INVERTED &&
InvertedIndexUtil.isVariantRootIndex(properties);
+ }
+
+ /**
+ * Validates the user's spelling of a VARIANT root index as given. Both
spellings of the
+ * scope must individually be the values scope (an invalid one is never
hidden by a valid
+ * sibling), and only the format version this FE stamps is accepted back,
so that a SHOW
+ * CREATE TABLE output replays. Canonicalization happens in
translateToCatalogStyle().
+ */
+ private void validateVariantRootIndex(DataType columnType, KeysType
keysType,
+ TInvertedIndexFileStorageFormat invertedIndexFileStorageFormat) {
+ String scope =
properties.get(InvertedIndexUtil.VARIANT_INDEX_SCOPE_KEY);
+ String mode = properties.get(InvertedIndexUtil.VARIANT_INDEX_MODE_KEY);
+ String formatVersion =
properties.get(InvertedIndexUtil.VARIANT_ROOT_FORMAT_VERSION_KEY);
+ if (scope == null && mode == null) {
+ if (formatVersion != null) {
+ throw new AnalysisException(
+ "variant_root_format_version requires
variant_index_scope");
+ }
+ return;
+ }
+ if (scope != null &&
InvertedIndexProperties.normalizeVariantIndexScope(scope) == null) {
+ throw new AnalysisException("variant_index_scope must be values,
found: " + scope);
+ }
+ if (mode != null &&
InvertedIndexProperties.normalizeVariantIndexScope(mode) == null) {
+ throw new AnalysisException("variant_index_mode must be
all_values, found: " + mode);
+ }
+ if (!columnType.isVariantType()) {
+ throw new AnalysisException(
+ "variant_index_scope=values can only be used on VARIANT
columns");
+ }
+ // Declared Decimal256 can be stored, but full-root assembly cannot
decode it.
+ // Reject this combination before accepting a table whose legal
inserts would fail.
+ VariantType variantType = (VariantType) columnType;
+ if (variantType.getPredefinedFields().stream()
+ .anyMatch(field -> containsDecimal256(field.getDataType()))) {
+ throw new AnalysisException("VARIANT values index does not support
declared Decimal256 paths");
+ }
+ if (invertedIndexFileStorageFormat !=
TInvertedIndexFileStorageFormat.SNII) {
Review Comment:
[P2] Preserve SNII when changing the future partition format
In cloud mode with
`enable_partition_inverted_index_storage_format_rollout=true`, a partitioned
table created with this values index and SNII still accepts `ALTER TABLE t SET
("partition.inverted_index_storage_format"="V2")` (also V3).
`CloudSchemaChangeHandler` only parses/persists that target, and newly added
partitions inherit the table's values index while their tablets use V2/V3.
Inserts into those partitions then fail in `init_variant_root_index_writers()`
with `VARIANT root index requires SNII storage format`. Enforce this format
invariant when altering the future partition default too, and cover
adding/loading a partition after the change.
##########
be/src/storage/compaction/compaction.cpp:
##########
@@ -135,6 +136,135 @@ namespace {
constexpr size_t kSniiCompactionReadAheadBudgetBytes = 64ULL << 20;
+using SniiSourceFileReaders = std::map<std::string,
std::unique_ptr<IndexFileReader>>;
+
+Result<IndexFileReader*> open_snii_compaction_source(const io::FileSystemSPtr&
fs,
+ const std::string& prefix,
+ const
InvertedIndexFileInfo& file_info,
+ int64_t tablet_id,
+ SniiSourceFileReaders&
readers) {
+ auto it = readers.find(prefix);
+ if (it == readers.end()) {
+ auto reader = std::make_unique<IndexFileReader>(
+ fs, prefix, InvertedIndexStorageFormatPB::SNII, file_info,
tablet_id);
+ const auto status =
reader->init(config::inverted_index_read_buffer_size);
+ if (status.is<ErrorCode::INVERTED_INDEX_FILE_NOT_FOUND>()) {
+ return ResultError(Status::Error<INVERTED_INDEX_NOT_SUPPORTED>(
+ "source SNII index file is unavailable"));
+ }
+ // Missing postings allow rebuild; actual IO failures and corrupt
containers abort.
+ RETURN_IF_ERROR_RESULT(status);
+ DBUG_EXECUTE_IF("Compaction::snii_eligibility_reader_initialized",
DBUG_RUN_CALLBACK());
+ it = readers.emplace(prefix, std::move(reader)).first;
+ }
+ return it->second.get();
+}
+
+Status validate_snii_merge_source(
+ const snii::reader::LogicalIndexReader& reader, const TabletIndex&
source_meta,
+ const TabletIndex& destination, size_t ordinal,
+ std::optional<snii::compaction::SniiCompactionEligibility>&
eligibility) {
+ if (!eligibility.has_value()) {
+ const std::array<snii::compaction::PlainT2CompactionSource, 1> source
= {
+ snii::compaction::PlainT2CompactionSource {.reader =
std::cref(reader),
+ .index_meta =
std::cref(source_meta)}};
+ snii::compaction::SniiCompactionEligibility result;
+
RETURN_IF_ERROR(snii::compaction::validate_snii_compaction_eligibility(source,
destination,
+
&result));
+ eligibility = result;
+ return Status::OK();
+ }
+ if (source_meta.index_id() != destination.index_id() ||
+ source_meta.get_index_suffix() != destination.get_index_suffix() ||
+ source_meta.properties() != destination.properties()) {
+ return Status::Error<INVERTED_INDEX_NOT_SUPPORTED>(
+ "source SNII index identity or properties differ from
destination");
+ }
+ return snii::compaction::validate_snii_source_eligibility(reader, ordinal,
*eligibility);
+}
+
+Result<uint64_t> check_snii_merge_sources(const std::vector<RowsetSharedPtr>&
rowsets,
+ const TabletIndex& destination,
int64_t tablet_id,
+ SniiSourceFileReaders& readers) {
+ size_t segment_count = 0;
+ for (const auto& rowset : rowsets) {
+ segment_count += rowset->num_segments();
+ }
+ if (segment_count == 0 ||
+ segment_count >
+ kSniiCompactionReadAheadBudgetBytes /
+
snii::compaction::SniiPlainT2MergePlan::kMinReadAheadBudgetPerSource) {
+ return ResultError(Status::Error<INVERTED_INDEX_NOT_SUPPORTED>(
+ "source index unavailable or read-ahead budget gate failed"));
+ }
+ const int32_t uid = destination.col_unique_ids().front();
+ std::optional<snii::compaction::SniiCompactionEligibility> eligibility;
+ size_t ordinal = 0;
+ // Guaranteed merge allocations: read-ahead plus one ubiquitous term's
docids/frequencies
+ // and the NULL docids from each source. Positions retain the existing
streamed accounting.
+ uint64_t estimated_bytes = kSniiCompactionReadAheadBudgetBytes;
+ for (const auto& rowset : rowsets) {
+ if
(static_cast<BetaRowset*>(rowset.get())->is_skip_index_compaction(uid)) {
+ return ResultError(Status::Error<INVERTED_INDEX_NOT_SUPPORTED>(
+ "source SNII index compaction is disabled"));
+ }
+ const auto source_metas =
rowset->tablet_schema()->inverted_indexs(uid);
+ const auto source_it = std::ranges::find_if(source_metas, [&](const
TabletIndex* index) {
+ return index->index_id() == destination.index_id();
+ });
+ const auto fs = rowset->rowset_meta()->fs();
+ if (source_it == source_metas.end() || fs == nullptr) {
+ return ResultError(Status::Error<INVERTED_INDEX_NOT_SUPPORTED>(
+ "source SNII index metadata or file system is
unavailable"));
+ }
+ for (auto segment : rowset->segments()) {
+ const auto path = segment.path();
+ if (!path.has_value()) {
+ return ResultError(Status::Error<INVERTED_INDEX_NOT_SUPPORTED>(
+ "source SNII segment path is unavailable"));
+ }
+ const std::string prefix
{InvertedIndexDescriptor::get_index_file_path_prefix(*path)};
+ const auto file_reader = open_snii_compaction_source(
+ fs, prefix, segment.inverted_index_file_info(), tablet_id,
readers);
+ if (!file_reader.has_value()) {
+ return ResultError(file_reader.error());
+ }
+ auto source =
+ (*file_reader)
+ ->open_snii_index(*source_it, nullptr,
+
snii::reader::LogicalIndexOpenMode::kCompaction);
+ if (!source.has_value() &&
+ source.error().is<ErrorCode::INVERTED_INDEX_FILE_NOT_FOUND>())
{
Review Comment:
[P2] Rebuild when a logical SNII index is absent
`open_snii_index()` returns `INVERTED_INDEX_SNII_NOT_FOUND` for an absent
`(index_id, suffix)` directory entry, so this check misses the actual
missing-logical-index status. With supported existing
`skip_write_index_on_load=true` metadata, direct loads create a valid SNII
container while omitting its logical indexes; container initialization succeeds
and reaches this branch. `construct_snii_index_compaction_columns()` now
propagates the error and repeatedly aborts normal compaction instead of
rebuilding the omitted postings, as the previous source-open fallback did.
Fresh CREATE/ALTER cannot enable this deprecated flag, but image replay
preserves existing true metadata and the BE still supports its load path. Treat
`SNII_NOT_FOUND` as rebuild eligibility alongside `FILE_NOT_FOUND`, retain real
IO/corruption failures, and cover a valid container missing a logical index.
--
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]
