github-actions[bot] commented on code in PR #60901:
URL: https://github.com/apache/doris/pull/60901#discussion_r2888760404
##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/KafkaRoutineLoadJob.java:
##########
@@ -953,20 +955,26 @@ public Long totalLag() {
@Override
public NereidsRoutineLoadTaskInfo toNereidsRoutineLoadTaskInfo() throws
UserException {
Expression deleteCondition = getDeleteCondition() != null
- ?
NereidsLoadUtils.parseExpressionSeq(getDeleteCondition().toSql()).get(0)
+ ? NereidsLoadUtils.parseExpressionSeq(
+
getDeleteCondition().accept(ExprToExternalSqlVisitor.INSTANCE,
+ ToSqlParams.WITHOUT_TABLE)).get(0)
Review Comment:
**Behavioral change**: The old code was `getDeleteCondition().toSql()` which
preserves table name prefixes (equivalent to `ExprToSqlVisitor.INSTANCE`). The
new code uses `ToSqlParams.WITHOUT_TABLE` which strips them
(`disableTableName=true`). This changes the SQL string passed to
`parseExpressionSeq()`.
Same issue applies to `getPrecedingFilter()` on line 965 below.
The `whereExpr` and `desc.getExpr()` calls below correctly use
`WITHOUT_TABLE` since they previously called `toSqlWithoutTbl()`.
Suggested fix: use `ExprToSqlVisitor.INSTANCE` with `null` context for these
two, or use `ExprToExternalSqlVisitor` with a `ToSqlParams` that has
`disableTableName=false`.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/InPredicate.java:
##########
@@ -45,7 +43,7 @@ public class InPredicate extends Predicate {
private static final String IN_ITERATE = "in_iterate";
private static final String NOT_IN_ITERATE = "not_in_iterate";
@SerializedName("ini")
- private boolean isNotIn;
+ boolean isNotIn;
Review Comment:
**Nit**: `isNotIn` was changed from `private` to package-private for visitor
access, but `InPredicate` already has a public getter `isNotIn()`. The visitor
could use the getter instead to preserve encapsulation. Same applies to
`IsNullPredicate.isNotNull` (has `isNotNull()`), `LikePredicate.op` (has
`getOp()`), `MatchPredicate.op` (has `getOp()`), and `SlotRef.desc` (has
`getDesc()`).
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/CompoundPredicate.java:
##########
@@ -143,6 +123,13 @@ public int hashCode() {
@Override
public String toString() {
- return toSqlImpl();
+ if (getChildren().size() == 1) {
+ Preconditions.checkState(getOp() == Operator.NOT);
+ return "NOT " + getChild(0).accept(ExprToSqlVisitor.INSTANCE,
null);
Review Comment:
**Nit**: This `toString()` body duplicates the logic in
`ExprToSqlVisitor.visitCompoundPredicate()`. It could be simplified to:
```java
return accept(ExprToSqlVisitor.INSTANCE, null);
```
Same applies to `ArithmeticExpr.toString()`.
--
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]
