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


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExecuteCommand.java:
##########
@@ -182,6 +195,36 @@ public void run(ConnectContext ctx, StmtExecutor executor) 
throws Exception {
         }
     }
 
+    static boolean canRebindShortCircuitPlaceholders(PrepareCommand command, 
StatementContext context) {
+        // Direct execution only updates key predicates, never serialized 
output expressions.
+        // Inspect the retained unbound filters as comparison slots also 
include SELECT expressions.
+        Set<PlaceholderId> filterParameters = new HashSet<>();
+        for (LogicalPlan filter : 
command.getLogicalPlan().<LogicalPlan>collectToList(
+                LogicalFilter.class::isInstance)) {
+            for (Expression conjunct : ((LogicalFilter<?>) 
filter).getConjuncts()) {
+                if (conjunct instanceof EqualTo) {
+                    EqualTo equality = (EqualTo) conjunct;
+                    if (equality.left() instanceof Placeholder && 
equality.right() instanceof UnboundSlot) {
+                        filterParameters.add(((Placeholder) 
equality.left()).getPlaceholderId());
+                    } else if (equality.right() instanceof Placeholder && 
equality.left() instanceof UnboundSlot) {
+                        filterParameters.add(((Placeholder) 
equality.right()).getPlaceholderId());
+                    }
+                }
+            }
+        }
+        Set<String> keyColumns = new HashSet<>();
+        for (Placeholder placeholder : command.getPlaceholders()) {

Review Comment:
   [P1] Do not rebind fixed predicates on the same key column
   
   `canRebindShortCircuitPlaceholders` accepts `WHERE pk = ? AND pk = 1` 
because the placeholder is a key filter and the key-column set contains one 
name. On the direct path, `PointQueryExecutor.updateScanNodeConjuncts` rewrites 
every cached binary predicate whose slot has that name, so rebinding the 
parameter to `2` changes the fixed `pk = 1` predicate to `pk = 2` and returns a 
row that the original query must reject. Restrict direct reuse to predicates 
that were placeholders (or reject any key with additional fixed equalities), 
and add this regression.



##########
be/src/service/point_query_executor.cpp:
##########
@@ -128,13 +128,91 @@ static void extract_slot_ref(const VExprSPtr& expr, 
TupleDescriptor* tuple_desc,
     }
 }
 
+Status Reusable::_validate_slot_schema(const TabletSchema& schema) const {
+    for (const auto* slot : tuple_desc()->slots()) {
+        // Never reinterpret an unset or stale UID through a same-name 
replacement column.

Review Comment:
   [P1] Reject unknown UIDs that may refer to dropped columns
   
   The new `new_column` branch accepts any positive UID that is absent from the 
current tablet schema when no active column has the same name. After a 
light-schema drop, the current schema intentionally omits dropped-column 
history, so an old prepared descriptor such as `(v1, uid=17)` is 
indistinguishable here from an FE-ahead addition. The row-store decoder then 
fills the descriptor default for the missing UID (or can decode an old rowset 
payload), returning a false-success value for a dropped column. Carry 
schema-generation/provenance evidence or reject unknown UIDs after checking the 
producing rowset; add a drop/old-prepared-plan regression while preserving the 
genuine FE-ahead default path.



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