This is an automated email from the ASF dual-hosted git repository.

gortiz 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 8190e43747f Re-run query optimizer after expression override for 
non-hybrid tables (#20148)
8190e43747f is described below

commit 8190e43747f40a88f2ae46c0f7d20caf86f2b395
Author: Manak Bisht <[email protected]>
AuthorDate: Thu Nov 5 19:24:23 2026 +0530

    Re-run query optimizer after expression override for non-hybrid tables 
(#20148)
---
 .../BaseSingleStageBrokerRequestHandler.java       |  12 +-
 .../BaseSingleStageBrokerRequestHandlerTest.java   | 122 +++++++++++++++++++++
 2 files changed, 132 insertions(+), 2 deletions(-)

diff --git 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
index 5f2bd682dcf..89aaeee8763 100644
--- 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
+++ 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java
@@ -734,8 +734,12 @@ public abstract class BaseSingleStageBrokerRequestHandler 
extends BaseBrokerRequ
     } else if (routeInfo.isOffline()) {
       // OFFLINE only
       setTableName(serverBrokerRequest, offlineTableName);
-      handleExpressionOverride(serverPinotQuery, 
_tableCache.getExpressionOverrideMap(offlineTableName));
+      Map<Expression, Expression> expressionOverrideMap = 
_tableCache.getExpressionOverrideMap(offlineTableName);
+      handleExpressionOverride(serverPinotQuery, expressionOverrideMap);
       handleTimestampIndexOverride(serverPinotQuery, offlineTableConfig);
+      if (expressionOverrideMap != null) {
+        _queryOptimizer.optimize(serverPinotQuery, schema);
+      }
       offlineBrokerRequest = serverBrokerRequest;
 
       requestContext.setFanoutType(RequestContext.FanoutType.OFFLINE);
@@ -743,8 +747,12 @@ public abstract class BaseSingleStageBrokerRequestHandler 
extends BaseBrokerRequ
     } else {
       // REALTIME only
       setTableName(serverBrokerRequest, realtimeTableName);
-      handleExpressionOverride(serverPinotQuery, 
_tableCache.getExpressionOverrideMap(realtimeTableName));
+      Map<Expression, Expression> expressionOverrideMap = 
_tableCache.getExpressionOverrideMap(realtimeTableName);
+      handleExpressionOverride(serverPinotQuery, expressionOverrideMap);
       handleTimestampIndexOverride(serverPinotQuery, realtimeTableConfig);
+      if (expressionOverrideMap != null) {
+        _queryOptimizer.optimize(serverPinotQuery, schema);
+      }
       realtimeBrokerRequest = serverBrokerRequest;
 
       requestContext.setFanoutType(RequestContext.FanoutType.REALTIME);
diff --git 
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandlerTest.java
 
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandlerTest.java
index 790090e9a39..deae4f9f903 100644
--- 
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandlerTest.java
+++ 
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandlerTest.java
@@ -1169,6 +1169,128 @@ public class BaseSingleStageBrokerRequestHandlerTest {
         "combined filter must retain the original WHERE clause, got literals: 
" + literalValues);
   }
 
+  /// When an expression override rewrites a nested predicate to a literal 
(e.g. `REGEXP_LIKE(foo, '.*') -> TRUE`),
+  /// the broker re-runs the query optimizer for offline-only and 
realtime-only tables so that the literal is removed
+  /// before routing, as it already does for hybrid tables. The override is 
applied after the initial optimization, and
+  /// a leftover `TRUE` operand would reach the broker segment pruners, which 
expect every filter node to be a function
+  /// call (e.g. `TimeSegmentPruner.getFilterTimeIntervals` would throw a 
`NullPointerException`).
+  @Test
+  public void 
testExpressionOverrideToLiteralIsOptimizedAwayForNonHybridTables()
+      throws Exception {
+    for (String tableNameWithType : List.of("mytable_OFFLINE", 
"mytable_REALTIME")) {
+      AtomicReference<BrokerRequest> capturedServerRequest = new 
AtomicReference<>();
+      handleRequestWithExpressionOverride(tableNameWithType, "TRUE",
+          "SELECT COUNT(*) FROM mytable WHERE ts >= 1000 AND REGEXP_LIKE(foo, 
'.*')", capturedServerRequest);
+      BrokerRequest serverRequest = capturedServerRequest.get();
+      Assert.assertNotNull(serverRequest, tableNameWithType + ": server query 
should have been captured");
+      Expression filter = serverRequest.getPinotQuery().getFilterExpression();
+      Assert.assertNotNull(filter, tableNameWithType + ": time predicate must 
be retained");
+      Function filterFunction = filter.getFunctionCall();
+      Assert.assertNotNull(filterFunction, tableNameWithType + ": filter must 
be a function");
+      Assert.assertEquals(filterFunction.getOperator(), FilterKind.AND.name());
+      for (Expression operand : filterFunction.getOperands()) {
+        Assert.assertNotNull(operand.getFunctionCall(),
+            tableNameWithType + ": overridden TRUE literal must be optimized 
away, got filter: " + filter);
+      }
+    }
+  }
+
+  /// When an expression override rewrites a predicate under an AND to 
`FALSE`, the re-optimized filter collapses to
+  /// `FALSE` and the broker returns an empty response without querying any 
server.
+  @Test
+  public void testExpressionOverrideToFalseSkipsServersForNonHybridTables()
+      throws Exception {
+    for (String tableNameWithType : List.of("mytable_OFFLINE", 
"mytable_REALTIME")) {
+      AtomicReference<BrokerRequest> capturedServerRequest = new 
AtomicReference<>();
+      BrokerResponse response = 
handleRequestWithExpressionOverride(tableNameWithType, "FALSE",
+          "SELECT COUNT(*) FROM mytable WHERE ts >= 1000 AND REGEXP_LIKE(foo, 
'.*')", capturedServerRequest);
+      Assert.assertNull(capturedServerRequest.get(),
+          tableNameWithType + ": always-false filter must not be sent to 
servers");
+      Assert.assertTrue(response.getExceptions().isEmpty(),
+          tableNameWithType + ": unexpected exceptions: " + 
response.getExceptions());
+    }
+  }
+
+  /// When an expression override rewrites a predicate under an OR to `TRUE`, 
the re-optimized filter collapses to
+  /// `TRUE` and the broker drops the filter entirely.
+  @Test
+  public void 
testExpressionOverrideToTrueUnderOrDropsFilterForNonHybridTables()
+      throws Exception {
+    for (String tableNameWithType : List.of("mytable_OFFLINE", 
"mytable_REALTIME")) {
+      AtomicReference<BrokerRequest> capturedServerRequest = new 
AtomicReference<>();
+      handleRequestWithExpressionOverride(tableNameWithType, "TRUE",
+          "SELECT COUNT(*) FROM mytable WHERE ts >= 1000 OR REGEXP_LIKE(foo, 
'.*')", capturedServerRequest);
+      BrokerRequest serverRequest = capturedServerRequest.get();
+      Assert.assertNotNull(serverRequest, tableNameWithType + ": server query 
should have been captured");
+      Assert.assertNull(serverRequest.getPinotQuery().getFilterExpression(),
+          tableNameWithType + ": always-true filter must be dropped");
+    }
+  }
+
+  private static BrokerResponse handleRequestWithExpressionOverride(String 
tableNameWithType, String overrideValue,
+      String sql, AtomicReference<BrokerRequest> capturedServerRequest)
+      throws Exception {
+    String rawTable = "mytable";
+    Schema schema = new Schema.SchemaBuilder()
+        .setSchemaName(rawTable)
+        .addSingleValueDimension("foo", DataType.STRING)
+        .addDateTime("ts", DataType.LONG, "1:MILLISECONDS:EPOCH", 
"1:MILLISECONDS")
+        .build();
+
+    TableCache tableCache = mock(TableCache.class);
+    when(tableCache.getActualTableName(rawTable)).thenReturn(rawTable);
+    when(tableCache.getSchema(rawTable)).thenReturn(schema);
+    when(tableCache.getColumnNameMap(anyString())).thenReturn(Map.of("foo", 
"foo", "ts", "ts"));
+    TableConfig tableCfg = mock(TableConfig.class);
+    when(tableCfg.getTenantConfig()).thenReturn(new TenantConfig("t_BROKER", 
"t_SERVER", null));
+    when(tableCache.getTableConfig(tableNameWithType)).thenReturn(tableCfg);
+    when(tableCache.getExpressionOverrideMap(tableNameWithType)).thenReturn(
+        Map.of(CalciteSqlParser.compileToExpression("REGEXP_LIKE(foo, '.*')"),
+            CalciteSqlParser.compileToExpression(overrideValue)));
+
+    BrokerRoutingManager routingManager = mock(BrokerRoutingManager.class);
+    when(routingManager.routingExists(tableNameWithType)).thenReturn(true);
+    when(routingManager.getQueryTimeoutMs(anyString())).thenReturn(10000L);
+    RoutingTable rt = mock(RoutingTable.class);
+    when(rt.getServerInstanceToSegmentsMap()).thenReturn(
+        Map.of(new ServerInstance(new InstanceConfig("server01_9000")),
+            new SegmentsToQuery(List.of("seg01"), List.of())));
+    when(routingManager.getRoutingTable(any(), 
Mockito.anyLong())).thenReturn(rt);
+
+    QueryQuotaManager quotaManager = mock(QueryQuotaManager.class);
+    when(quotaManager.acquire(anyString())).thenReturn(true);
+    when(quotaManager.acquireDatabase(anyString())).thenReturn(true);
+    when(quotaManager.acquireApplication(anyString())).thenReturn(true);
+
+    BrokerMetrics.register(mock(BrokerMetrics.class));
+    PinotConfiguration config = new PinotConfiguration();
+    BrokerQueryEventListenerFactory.init(config);
+
+    BaseSingleStageBrokerRequestHandler handler =
+        new BaseSingleStageBrokerRequestHandler(config, "broker1", new 
BrokerRequestIdGenerator(),
+            routingManager, new AllowAllAccessControlFactory(), quotaManager, 
tableCache,
+            ThreadAccountantUtils.getNoOpAccountant(), null) {
+          @Override
+          public void start() {
+          }
+
+          @Override
+          public void shutDown() {
+          }
+
+          @Override
+          protected BrokerResponseNative processBrokerRequest(long requestId,
+              BrokerRequest originalBrokerRequest, BrokerRequest 
serverBrokerRequest,
+              TableRouteInfo route, long timeoutMs, ServerStats serverStats,
+              RequestContext requestContext) {
+            capturedServerRequest.set(serverBrokerRequest);
+            return BrokerResponseNative.empty();
+          }
+        };
+
+    return handler.handleRequest(sql);
+  }
+
   /// Pins the security-style defense that a user-supplied 
`materializedViewRewrite=true` query
   /// option (e.g. via `SET materializedViewRewrite='true'`) is stripped at 
the broker entry
   /// before any compile work. Without the strip, a hostile client could stamp 
the


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to