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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
