You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ha...@apache.org on 2026/04/23 16:43:35 UTC
(pinot) branch master updated: Revert "Fix false-positive "column deleted" stale check for timestamp-index d…" (#18309)
This is an automated email from the ASF dual-hosted git repository.
noob-se7en pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new b11fe9ef5ee Revert "Fix false-positive "column deleted" stale check for timestamp-index d…" (#18309)
b11fe9ef5ee is described below
commit b11fe9ef5ee9a92c8dfe6c797c3691c9ec530549
Author: Gonzalo Ortiz Jaureguizar <go...@users.noreply.github.com>
AuthorDate: Thu Apr 23 18:43:29 2026 +0200
Revert "Fix false-positive "column deleted" stale check for timestamp-index d…" (#18309)
This reverts commit 6da39b46164668dee4ebe99537e5ec8ec0a30025.
---
.../core/data/manager/BaseTableDataManager.java | 15 ---
.../BaseTableDataManagerNeedRefreshTest.java | 133 ---------------------
2 files changed, 148 deletions(-)
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
index 4ffdf6a1538..ea46c9320f0 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
@@ -116,7 +116,6 @@ import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.ConsumingSegmentConsistencyModeListener;
-import org.apache.pinot.spi.utils.TimestampIndexUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -1631,20 +1630,6 @@ public abstract class BaseTableDataManager implements TableDataManager {
// Column is deleted
if (fieldSpecInSchema == null) {
- // Timestamp-index derived columns ($col$GRANULARITY) are materialized in the segment when the timestamp
- // index is configured, but are not present in the table schema as first-class columns. If the timestamp
- // index is later removed from the table config, applyTimestampIndex no longer injects the derived column
- // into the IndexLoadingConfig schema, causing a false "column deleted" stale verdict. These columns are
- // managed by the timestamp index staleness check (via "column added" when re-enabling) and by
- // SegmentPreProcessor at reload time, so skipping them here is safe.
- // We only skip when the base column is still present in the schema; if the base column was also removed,
- // the column is not a timestamp-derived orphan and must still trigger "column deleted".
- if (TimestampIndexUtils.isValidColumnWithGranularity(columnName)) {
- String baseColumn = columnName.substring(1, columnName.indexOf('$', 1));
- if (schema.getFieldSpecFor(baseColumn) != null) {
- continue;
- }
- }
LOGGER.debug("tableNameWithType: {}, columnName: {}, segmentName: {}, change: column deleted",
tableNameWithType, columnName, segmentName);
return new StaleSegment(segmentName, true, "column deleted: " + columnName);
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerNeedRefreshTest.java b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerNeedRefreshTest.java
index 522b96a8364..a5159547079 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerNeedRefreshTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerNeedRefreshTest.java
@@ -43,8 +43,6 @@ import org.apache.pinot.spi.config.table.StarTreeAggregationConfig;
import org.apache.pinot.spi.config.table.StarTreeIndexConfig;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableType;
-import org.apache.pinot.spi.config.table.TimestampConfig;
-import org.apache.pinot.spi.config.table.TimestampIndexGranularity;
import org.apache.pinot.spi.data.DimensionFieldSpec;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.MetricFieldSpec;
@@ -862,135 +860,4 @@ public class BaseTableDataManagerNeedRefreshTest {
BASE_TABLE_DATA_MANAGER.isSegmentStale(new IndexLoadingConfig(newTableConfig, SCHEMA), segmentDataManager)
.isStale());
}
-
- @Test
- void testTimestampIndexNoChange()
- throws Exception {
- // Segment built with a DAY timestamp index; table config still has the same timestamp index → not stale.
- FieldConfig fieldConfig = new FieldConfig.Builder(MS_SINCE_EPOCH_COLUMN_NAME)
- .withTimestampConfig(new TimestampConfig(List.of(TimestampIndexGranularity.DAY)))
- .build();
- TableConfig tableConfig = getTableConfigBuilder().setFieldConfigList(List.of(fieldConfig)).build();
- // Use a fresh schema per test: applyTimestampIndex mutates the schema object in-place, so sharing the
- // static SCHEMA across timestamp tests causes cross-test contamination.
- IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, getSchema());
- ImmutableSegmentDataManager segmentDataManager =
- createImmutableSegmentDataManager(indexLoadingConfig, _testName, generateRows());
-
- assertFalse(BASE_TABLE_DATA_MANAGER.isSegmentStale(indexLoadingConfig, segmentDataManager).isStale());
- }
-
- @Test
- void testTimestampIndexAdded()
- throws Exception {
- // Segment built without a timestamp index; table config adds a DAY timestamp index → stale ("column added").
- FieldConfig fieldConfig = new FieldConfig.Builder(MS_SINCE_EPOCH_COLUMN_NAME)
- .withTimestampConfig(new TimestampConfig(List.of(TimestampIndexGranularity.DAY)))
- .build();
- TableConfig tableConfigWithTimestampIndex =
- getTableConfigBuilder().setFieldConfigList(List.of(fieldConfig)).build();
- IndexLoadingConfig indexLoadingConfigWithTimestampIndex =
- new IndexLoadingConfig(tableConfigWithTimestampIndex, getSchema());
-
- // Segment was created without the timestamp index.
- StaleSegment response =
- BASE_TABLE_DATA_MANAGER.isSegmentStale(indexLoadingConfigWithTimestampIndex, IMMUTABLE_SEGMENT_DATA_MANAGER);
- assertTrue(response.isStale());
- assertEquals(response.getReason(), "column added");
- }
-
- @Test
- void testTimestampIndexRemoved()
- throws Exception {
- // Segment built with a DAY timestamp index; table config no longer has the timestamp index → not stale.
- // This tests the fix: the derived $col$DAY column in the segment must not be flagged as "column deleted".
- FieldConfig fieldConfig = new FieldConfig.Builder(MS_SINCE_EPOCH_COLUMN_NAME)
- .withTimestampConfig(new TimestampConfig(List.of(TimestampIndexGranularity.DAY)))
- .build();
- TableConfig tableConfigWithTimestampIndex =
- getTableConfigBuilder().setFieldConfigList(List.of(fieldConfig)).build();
- IndexLoadingConfig indexLoadingConfigWithTimestampIndex =
- new IndexLoadingConfig(tableConfigWithTimestampIndex, getSchema());
- ImmutableSegmentDataManager segmentDataManager =
- createImmutableSegmentDataManager(indexLoadingConfigWithTimestampIndex, _testName, generateRows());
-
- // Evaluate staleness against a fresh config with no timestamp index.
- IndexLoadingConfig indexLoadingConfigNoTimestamp = new IndexLoadingConfig(getTableConfigBuilder().build(),
- getSchema());
- assertFalse(BASE_TABLE_DATA_MANAGER.isSegmentStale(indexLoadingConfigNoTimestamp, segmentDataManager).isStale());
- }
-
- @Test
- void testTimestampIndexGranularityAdded()
- throws Exception {
- // Segment built with a DAY timestamp index; table config adds a WEEK granularity → stale ("column added").
- FieldConfig fieldConfigDay = new FieldConfig.Builder(MS_SINCE_EPOCH_COLUMN_NAME)
- .withTimestampConfig(new TimestampConfig(List.of(TimestampIndexGranularity.DAY)))
- .build();
- TableConfig tableConfigDay = getTableConfigBuilder().setFieldConfigList(List.of(fieldConfigDay)).build();
- IndexLoadingConfig indexLoadingConfigDay = new IndexLoadingConfig(tableConfigDay, getSchema());
- ImmutableSegmentDataManager segmentDataManager =
- createImmutableSegmentDataManager(indexLoadingConfigDay, _testName, generateRows());
-
- FieldConfig fieldConfigDayAndWeek = new FieldConfig.Builder(MS_SINCE_EPOCH_COLUMN_NAME)
- .withTimestampConfig(
- new TimestampConfig(List.of(TimestampIndexGranularity.DAY, TimestampIndexGranularity.WEEK)))
- .build();
- TableConfig tableConfigDayAndWeek =
- getTableConfigBuilder().setFieldConfigList(List.of(fieldConfigDayAndWeek)).build();
- IndexLoadingConfig indexLoadingConfigDayAndWeek = new IndexLoadingConfig(tableConfigDayAndWeek, getSchema());
-
- StaleSegment response = BASE_TABLE_DATA_MANAGER.isSegmentStale(indexLoadingConfigDayAndWeek, segmentDataManager);
- assertTrue(response.isStale());
- assertEquals(response.getReason(), "column added");
- }
-
- @Test
- void testTimestampIndexGranularityRemoved()
- throws Exception {
- // Segment built with DAY + WEEK timestamp index; table config drops WEEK → not stale.
- // The orphaned $col$WEEK column in the segment must not be flagged as "column deleted".
- FieldConfig fieldConfigDayAndWeek = new FieldConfig.Builder(MS_SINCE_EPOCH_COLUMN_NAME)
- .withTimestampConfig(
- new TimestampConfig(List.of(TimestampIndexGranularity.DAY, TimestampIndexGranularity.WEEK)))
- .build();
- TableConfig tableConfigDayAndWeek =
- getTableConfigBuilder().setFieldConfigList(List.of(fieldConfigDayAndWeek)).build();
- IndexLoadingConfig indexLoadingConfigDayAndWeek = new IndexLoadingConfig(tableConfigDayAndWeek, getSchema());
- ImmutableSegmentDataManager segmentDataManager =
- createImmutableSegmentDataManager(indexLoadingConfigDayAndWeek, _testName, generateRows());
-
- FieldConfig fieldConfigDay = new FieldConfig.Builder(MS_SINCE_EPOCH_COLUMN_NAME)
- .withTimestampConfig(new TimestampConfig(List.of(TimestampIndexGranularity.DAY)))
- .build();
- TableConfig tableConfigDay = getTableConfigBuilder().setFieldConfigList(List.of(fieldConfigDay)).build();
- IndexLoadingConfig indexLoadingConfigDay = new IndexLoadingConfig(tableConfigDay, getSchema());
-
- assertFalse(BASE_TABLE_DATA_MANAGER.isSegmentStale(indexLoadingConfigDay, segmentDataManager).isStale());
- }
-
- @Test
- void testUserDefinedColumnWithGranularityPatternDeleted()
- throws Exception {
- // A user-defined column whose name matches $col$GRANULARITY must still trigger "column deleted" when
- // both the column and its apparent base column are absent from the new schema.
- // The skip guard only fires when the base column is still in the schema (timestamp-derived orphan case);
- // when the base column is also gone, the skip must not activate.
- String baseColumnName = "customBase";
- String granularityLookingColumn = "$customBase$DAY";
- Schema schemaWithBothColumns = getSchema();
- schemaWithBothColumns.addField(new DimensionFieldSpec(baseColumnName, FieldSpec.DataType.STRING, true));
- schemaWithBothColumns.addField(new DimensionFieldSpec(granularityLookingColumn, FieldSpec.DataType.STRING, true));
- IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(getTableConfigBuilder().build(),
- schemaWithBothColumns);
- ImmutableSegmentDataManager segmentDataManager =
- createImmutableSegmentDataManager(indexLoadingConfig, _testName, generateRows());
-
- // New schema has neither the derived column nor its base column.
- // The skip guard must not activate, so "column deleted" fires for the $-prefixed column (alphabetically first).
- StaleSegment response = BASE_TABLE_DATA_MANAGER.isSegmentStale(
- new IndexLoadingConfig(getTableConfigBuilder().build(), getSchema()), segmentDataManager);
- assertTrue(response.isStale());
- assertEquals(response.getReason(), "column deleted: " + granularityLookingColumn);
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org