github-actions[bot] commented on code in PR #68227:
URL: https://github.com/apache/doris/pull/68227#discussion_r4068085726


##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/MetadataGenerator.java:
##########
@@ -2329,8 +2337,68 @@ private static TFetchSchemaTableDataResult 
streamMetadataResult(TSchemaTableRequ
     private static TFetchSchemaTableDataResult 
streamConsumptionMetadataResult(TSchemaTableRequestParams params) {
         TFetchSchemaTableDataResult result = new TFetchSchemaTableDataResult();
         List<TRow> dataBatch = Lists.newArrayList();
+        // Decode the planner predicates carried back by BE. Conversion 
failures only disable FE pruning.
+        List<Expression> parsedConjuncts = Collections.emptyList();
+        if (params.isSetFrontendConjuncts()) {
+            try {
+                parsedConjuncts = 
FrontendConjunctsUtils.convertToExpression(params.getFrontendConjuncts());
+            } catch (RuntimeException e) {
+                LOG.warn("Failed to convert frontend conjuncts for 
table_stream_consumption; skip FE pruning", e);
+            }
+        }
+        List<Expression> conjuncts = parsedConjuncts;
         try {
-            
Env.getCurrentEnv().getTableStreamManager().fillStreamConsumptionValuesMetadataResult(dataBatch);
+            // Keep unfiltered scans on the direct path without allocating a 
selector or partition snapshots.
+            if (conjuncts.isEmpty()) {
+                
Env.getCurrentEnv().getTableStreamManager().fillStreamConsumptionValuesMetadataResult(dataBatch);
+                result.setDataBatch(dataBatch);
+                result.setStatus(new TStatus(TStatusCode.OK));
+                return result;
+            }
+            // Split predicates by the earliest metadata level that has every 
referenced column.
+            // Stream-only predicates run once per stream; predicates using 
UNIT run once per partition.
+            List<Expression> streamConjuncts = Lists.newArrayList();
+            List<Expression> unitConjuncts = Lists.newArrayList();
+            for (Expression conjunct : conjuncts) {
+                Set<String> referencedColumns = new HashSet<>();
+                for (UnboundSlot slot : 
conjunct.<UnboundSlot>collectToList(UnboundSlot.class::isInstance)) {
+                    List<String> nameParts = slot.getNameParts();
+                    if (!nameParts.isEmpty()) {
+                        referencedColumns.add(nameParts.get(nameParts.size() - 
1).toUpperCase(Locale.ROOT));
+                    }
+                }
+                // containsAll allows any subset, but rejects predicates that 
need unavailable columns.
+                if 
(STREAM_CONSUMPTION_STREAM_COLUMNS.containsAll(referencedColumns)) {
+                    streamConjuncts.add(conjunct);
+                } else if (referencedColumns.contains("UNIT")
+                        && 
STREAM_CONSUMPTION_SELECTOR_COLUMNS.containsAll(referencedColumns)) {
+                    unitConjuncts.add(conjunct);
+                }
+            }
+            // Bind each candidate's metadata values in the selector; 
unsupported predicates remain for BE filtering.
+            
Env.getCurrentEnv().getTableStreamManager().fillStreamConsumptionValuesMetadataResult(
+                    dataBatch, new 
TableStreamManager.StreamConsumptionSelector() {
+                        @Override
+                        public boolean test(String dbName, String streamName, 
long streamId, String unit) {
+                            List<Expression> currentConjuncts = unit == null ? 
streamConjuncts : unitConjuncts;
+                            if (currentConjuncts.isEmpty()) {
+                                return true;
+                            }
+                            TreeMap<String, Object> values = new 
TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+                            values.put("DB_NAME", dbName);
+                            values.put("STREAM_NAME", streamName);
+                            values.put("STREAM_ID", streamId);
+                            if (unit != null) {
+                                values.put("UNIT", unit);
+                            }
+                            return 
!FrontendConjunctsUtils.isFiltered(currentConjuncts, values);

Review Comment:
   [P1] Preserve query-session strict-cast semantics during FE pruning
   
   Reduced plan:
   ```text
   Filter(CAST(STREAM_NAME AS INT) = 1)
     LogicalSchemaScan(table_stream_consumption)
   ```
   With query-session `enable_strict_cast=true`, the root `EqualTo` is 
forwarded here, but this callback FE reparses an ordinary `Cast` and evaluates 
it without the query `ConnectContext`. `FoldConstantRuleOnFE.visitCast` 
therefore sees `cast.isStrict() == false` and falls back to the global default 
(normally false); a nonnumeric stream name becomes NULL and this return prunes 
the row. The retained BE predicate receives the query's strict option and would 
instead raise the required cast error, so pruning suppresses observable query 
semantics. This is distinct from the existing `NoneMovableFunction` thread 
because an explicit cast is movable. Please recursively restrict forwarded 
subtrees to context-independent expressions or propagate/evaluate the relevant 
session state, and add an end-to-end strict-cast error-preservation test.



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/stream/TableStreamManager.java:
##########
@@ -412,22 +439,43 @@ public void 
fillStreamConsumptionValuesMetadataResult(List<TRow> dataBatch) thro
                         }
                         continue;
                     }
-                    Preconditions.checkArgument(table.get() instanceof 
BaseTableStream);
-                    BaseTableStream stream = (BaseTableStream) table.get();
+                    Preconditions.checkArgument(table.get() instanceof 
OlapTableStream);
+                    OlapTableStream stream = (OlapTableStream) table.get();
+                    String dbName = db.get().getFullName();
+                    String streamName = stream.getName();
+                    long streamId = stream.getId();
+                    if (selector != null && !selector.test(dbName, streamName, 
streamId, null)) {

Review Comment:
   [P2] Snapshot the database name used for selection and output together
   
   This tests a `dbName` read without the database lock. If it reads `old_db` 
and a concurrent `ALTER DATABASE old_db RENAME new_db` then completes, 
`Database.setNameWithoutLock` updates both `fullQualifiedName` and every 
table's volatile `qualifiedDbName`. A `DB_NAME = 'new_db'` selector now rejects 
the stale value and skips the stream, although the unoptimized local path would 
later emit `new_db`; the cloud path likewise rereads `db.getFullName()` only 
after this pre-lock test. This is distinct from the existing 
selector-under-lock performance thread because it is a false-negative 
correctness interleaving. Please capture the database/stream identity under the 
protecting metadata locks and evaluate and materialize from that same snapshot.



-- 
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]

Reply via email to