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]