Logical tables MSE test by krishan1390 · Pull Request #17697 · apache/pinot

krishan1390 · 2026-02-13T10:51:18Z

Add multi-stage engine tests for logical tables.
Refactor few test classes (BaseLogicalTableIntegrationTest) to avoid duplicate code.

The primary gap is in "testBetween" test. The explain plan for a "not-between" query is different when queried via Logical tables vs without Logical tables. The results are consistent, but the explained plan is different. For now, I am filtering the validation for logical tables.

Explain plan for "not-between" query for logical table

PinotLogicalAggregate(group=[{}], agg#0=[COUNT($0)], aggType=[FINAL])
        PlanWithNoSegments(table=[mytable])

Explain plan for "not-between" query for physical table

  PinotLogicalAggregate(group=[{}], agg#0=[COUNT($0)], aggType=[FINAL])
    PinotLogicalExchange(distribution=[hash])
      LeafStageCombineOperator(table=[mytable])
        StreamingInstanceResponse
          CombineAggregate
            Aggregate(aggregations=[[count(*)]])
              Project(columns=[[]])
                DocIdSet(maxDocs=[120000])
                  FilterNot
                    FilterFullScan(predicate=[RandomAirports BETWEEN 'GTR' AND 'SUN'], operator=[RANGE])

codecov-commenter · 2026-02-13T11:39:17Z

❌ 3 Tests Failed:

Tests completed	Failed	Passed	Skipped
9381	3	9378	51

View the full list of 3 ❄️ flaky test(s)

org.apache.pinot.integration.tests.KafkaConfluentSchemaRegistryAvroMessageDecoderRealtimeClusterIntegrationTest::setUp
Flake rate in main: 100.00% (Passed 0 times, Failed 76 times)
Stack Traces | 12.4s run time
Could not find a valid Docker environment. Please see logs and check configuration

org.apache.pinot.plugin.inputformat.json.confluent.JsonConfluentSchemaTest::@BeforeClass setup
Flake rate in main: 100.00% (Passed 0 times, Failed 49 times)
Stack Traces | 0.491s run time
Could not find a valid Docker environment. Please see logs and check configuration

org.apache.pinot.plugin.inputformat.json.confluent.JsonConfluentSchemaTest::setup
Flake rate in main: 100.00% (Passed 0 times, Failed 98 times)
Stack Traces | 0.491s run time
Could not find a valid Docker environment. Please see logs and check configuration

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

krishan1390 · 2026-02-13T12:06:18Z

The test failures are due to the docker environment issue.

2026-02-13T11:57:03.9083814Z [ERROR]   KafkaConfluentSchemaRegistryAvroMessageDecoderRealtimeClusterIntegrationTest.setUp:292->BaseRealtimeClusterIntegrationTest.setUp:67->startKafka:88->startSchemaRegistry:99 » IllegalState Could not find a valid Docker environment. Please see logs and check configuration

This is unrealted to this PR

krishan1390 · 2026-02-13T12:11:21Z

...n-test-base/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java

    return getTableConfigBuilder(TableType.REALTIME).build();
  }

+  private List<String> getTimeBoundaryTable(List<String> offlineTables) {


These methods are moved from BaseLogicalTableIntegrationTest to this class so that LogicalTableMultiStageEngineIntegrationTest can also leverage them

…e_test

Copilot

Pull request overview

Adds multi-stage engine (MSE) integration coverage for logical tables and refactors shared logical-table test setup to reduce duplication, including factoring common “upload offline tables” and logical-table-config creation into the base integration test utilities.

Changes:

Add LogicalTableMultiStageEngineIntegrationTest to run MSE test suite against a logical table backed by multiple offline physical tables.
Refactor logical table integration test setup to reuse shared helpers for offline-table data upload and logical-table schema creation.
Refactor MultiStageEngineIntegrationTest setup/teardown into overridable hooks and make testBetween explain-plan validation configurable.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/logicaltable/LogicalTableMultiStageEngineIntegrationTest.java`	New MSE logical-table integration test; overrides setup, logical-table config, and selectively disables validations for logical tables.
`pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/logicaltable/BaseLogicalTableIntegrationTest.java`	Refactors logical-table test setup to reuse shared base helpers and separates logical-table schema creation from table creation.
`pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TlsIntegrationTest.java`	Adjusts logical-table config creation to support realtime-only logical table in TLS test setup.
`pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java`	Refactors setup into `initCluster/initTables/initOtherDependencies`, adds `getOfflineTablesCreated()`, and makes NOT BETWEEN plan validation optional.
`pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java`	Adds shared helpers: offline-table config overload, logical-table-config builder for custom physical sets, logical-table+schema helper, and offline upload/distribution utilities.

Copilot · 2026-02-13T21:17:56Z

...n-test-base/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java

+    return timeBoundaryTable != null ? List.of(TableNameBuilder.OFFLINE.tableNameWithType(timeBoundaryTable))
+        : List.of();


getTimeBoundaryTable() iterates over offlineTables and sets timeBoundaryTable = tableName, but then returns TableNameBuilder.OFFLINE.tableNameWithType(timeBoundaryTable). In current call sites (createLogicalTableConfig() and BaseLogicalTableIntegrationTest), offlineTables are already table-name-with-type (e.g. o_1_OFFLINE), so this produces an invalid name like o_1_OFFLINE_OFFLINE and breaks the includedTables in TimeBoundaryConfig.

Suggested change

return timeBoundaryTable != null ? List.of(TableNameBuilder.OFFLINE.tableNameWithType(timeBoundaryTable))

: List.of();

return timeBoundaryTable != null ? List.of(timeBoundaryTable) : List.of();

Copilot · 2026-02-13T21:17:57Z

...n-test-base/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java

+  protected void createLogicalTableAndSchema()
+      throws IOException {
+    Schema schema = createSchema(getSchemaFileName());
+    schema.setSchemaName(getTableName());


createLogicalTableAndSchema() sets the schema name to getTableName(), but the schema for a logical table should be created with the logical table name (i.e., getLogicalTableName()). If a subclass uses a logical table name different from the physical table name, the controller will not find the expected schema for the logical table.

Suggested change

schema.setSchemaName(getTableName());

schema.setSchemaName(getLogicalTableName());

Copilot · 2026-02-13T21:17:57Z

...apache/pinot/integration/tests/logicaltable/LogicalTableMultiStageEngineIntegrationTest.java

+  @Ignore
+  public void testValidateQueryApiBatchMixedResults() {
+  }
+
+  @Override
+  @Ignore
+  public void testValidateQueryApiSuccessfulQueries() {
+  }
+
+  @Override
+  @Ignore
+  public void testValidateQueryApiUnsuccessfulQueries() {
+  }
+
+  @Override
+  @Ignore


These overrides are annotated with @Ignore but not @Test. In TestNG, overriding a @Test method without @Test removes it from the test run entirely (it won't show as skipped/ignored). If the intent is to skip these inherited tests explicitly, add @Test (or @Test(enabled = false)) alongside @Ignore on each override.

Suggested change

@Ignore

public void testValidateQueryApiBatchMixedResults() {

}

@Override

@Ignore

public void testValidateQueryApiSuccessfulQueries() {

}

@Override

@Ignore

public void testValidateQueryApiUnsuccessfulQueries() {

}

@Override

@Ignore

@Ignore

@Test(enabled = false)

public void testValidateQueryApiBatchMixedResults() {

}

@Override

@Ignore

@Test(enabled = false)

public void testValidateQueryApiSuccessfulQueries() {

}

@Override

@Ignore

@Test(enabled = false)

public void testValidateQueryApiUnsuccessfulQueries() {

}

@Override

@Ignore

@Test(enabled = false)

Copilot · 2026-02-13T21:17:57Z

...apache/pinot/integration/tests/logicaltable/LogicalTableMultiStageEngineIntegrationTest.java

+  @BeforeClass
+  @Override
+  public void setUp()
+      throws Exception {
+    initCluster();
+    List<File> avroFiles = unpackAvroData(_tempDir);
+    initTables(avroFiles);
+    initOtherDependencies(avroFiles);
+  }


This setUp() override duplicates the exact initialization sequence already implemented in MultiStageEngineIntegrationTest.setUp(). Since the base setUp() calls overridable hooks (initTables, etc.), this override can be removed (or replaced with super.setUp()) to reduce duplication and the risk of future drift.

Suggested change

@BeforeClass

@Override

public void setUp()

throws Exception {

initCluster();

List<File> avroFiles = unpackAvroData(_tempDir);

initTables(avroFiles);

initOtherDependencies(avroFiles);

}

Copilot · 2026-02-13T21:17:57Z

...apache/pinot/integration/tests/logicaltable/LogicalTableMultiStageEngineIntegrationTest.java

+  @Override
+  @Test
+  public void testBetween()
+      throws Exception {
+    testBetween(false);
+    // TODO - Explain plan result is different for logical table vs physical table.
+    //  That is why we're overriding the test and avoiding explain plan validation.
+    //  This needs to be fixed
+  }


testBetween() in the logical-table suite skips the NOT BETWEEN explain-plan assertions entirely. This reduces coverage for an optimizer/planner-sensitive case and may allow regressions to slip in unnoticed. Consider validating a logical-table-appropriate invariant for the NOT BETWEEN plan (even if different from physical tables), or making the assertion accept both known plan shapes instead of disabling it.

yashmayya · 2026-02-13T21:49:24Z

...-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java

-    Assert.assertFalse(plan.contains(">="));
-    Assert.assertFalse(plan.contains("<="));
-    Assert.assertFalse(plan.contains("Sarg"));
+    if (testExplainPlanForNotBetween) {


I don't get this, why is NOT BETWEEN a special case here? Or is this the only one where the logical plan is being compared with the segment level plan and segment level explain plan for MSE with logical tables is broken?

krishan1390 added 3 commits February 13, 2026 12:28

Enable MSEIT for logical table by extending MSEIT

721761f

Refactor to avoid duplicate code

02c50a6

Refactor to avoid duplicate code

dc4ece9

Improve few tests

beecf80

krishan1390 commented Feb 13, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/master' into logical_tables_ms…

6bf0eeb

…e_test

krishan1390 changed the title ~~Logical tables mse test~~ Logical tables MSE test Feb 13, 2026

yashmayya requested review from Copilot, gortiz and yashmayya February 13, 2026 21:10

yashmayya added testing multi-stage Related to the multi-stage query engine logical-tables labels Feb 13, 2026

Copilot started reviewing on behalf of yashmayya February 13, 2026 21:11 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

yashmayya reviewed Feb 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logical tables MSE test#17697

Logical tables MSE test#17697
krishan1390 wants to merge 5 commits intoapache:masterfrom
krishan1390:logical_tables_mse_test

krishan1390 commented Feb 13, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Feb 13, 2026 •

edited

Loading

Uh oh!

krishan1390 commented Feb 13, 2026

Uh oh!

krishan1390 Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

yashmayya Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		return timeBoundaryTable != null ? List.of(TableNameBuilder.OFFLINE.tableNameWithType(timeBoundaryTable))
		: List.of();

	schema.setSchemaName(getTableName());
	schema.setSchemaName(getLogicalTableName());

Conversation

krishan1390 commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 3 Tests Failed:

Uh oh!

krishan1390 commented Feb 13, 2026

Uh oh!

krishan1390 Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

yashmayya Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

krishan1390 commented Feb 13, 2026 •

edited

Loading

codecov-commenter commented Feb 13, 2026 •

edited

Loading