Copilot commented on code in PR #58349:
URL: https://github.com/apache/doris/pull/58349#discussion_r2558846520
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java:
##########
@@ -250,11 +252,26 @@ public void executeQuery(String originStmt) throws
Exception {
if (stmts == null) {
stmts = parseWithFallback(originStmt, convertedStmt,
sessionVariable);
- if (stmts == null) {
+ if (stmts == null || stmts.isEmpty()) {
return;
}
}
+ // check cache hit
+ // This is only for test cases, to check if the sql cache is hit
correctly.
+ StatementBase checkStmt = stmts.get(0);
+ if (checkStmt instanceof LogicalPlanAdapter &&
wantToParseSqlFromSqlCache) {
+ // UnboundResultSink means this is a query stmt, and we only check
for query stmt here.
+ if (((LogicalPlanAdapter) checkStmt).getLogicalPlan() instanceof
UnboundResultSink
+ &&
ctx.getSessionVariable().testQueryCacheHit.equals("sql") && cachedStmts ==
null) {
Review Comment:
The code checks for `testQueryCacheHit.equals("sql")`, but the
SessionVariable definition in SessionVariable.java line 2369 specifies valid
options as `{"none", "sql_cache", "partition_cache"}`. The value `"sql"` is not
in the list of valid options. Either:
1. Change this check to use `"sql_cache"` instead of `"sql"`, OR
2. Update the SessionVariable annotation to include `"sql"` instead of or in
addition to `"sql_cache"`
This inconsistency will cause confusion for users and may cause validation
errors if there's a validator checking against the declared options.
```suggestion
&&
ctx.getSessionVariable().testQueryCacheHit.equals("sql_cache") && cachedStmts
== null) {
```
--
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]
