wenzhenghu commented on code in PR #63919:
URL: https://github.com/apache/doris/pull/63919#discussion_r3353087047


##########
fe/fe-core/src/test/java/org/apache/doris/qe/SessionVariablesTest.java:
##########
@@ -44,8 +47,7 @@ protected void runBeforeAll() throws Exception {
         FeConstants.runningUnitTest = true;
         createDatabase("test_d");
         useDatabase("test_d");
-        createTable("create table test_t1 \n" + "(k1 int, k2 int) distributed 
by hash(k1) buckets 1\n"
-                + "properties(\"replication_num\" = \"1\");");
+        // Skip creating an OLAP table because these cases only validate 
session-variable behavior and parsing.
 
         sessionVariable = new SessionVariable();
         Field[] fields = SessionVariable.class.getDeclaredFields();

Review Comment:
   > This setup removal breaks the existing `testSetVarInHint()` below. That 
test still parses `insert into test_t1 ... from test_t1 ...`; during parsing, 
`NereidsParser` applies the set-var hint against the current `ConnectContext` 
and resolves the statement in the test FE context. Without creating `test_t1` 
in `runBeforeAll()`, this class can now fail with an unknown-table analysis 
error before it reaches the new session-variable assertions. Please keep the 
fixture table creation or make `testSetVarInHint()` independent before removing 
it.
   
   Thanks for the review.
   
   This case only checks that `set_var(...)` is applied during 
`NereidsParser.parseSQL(sql)`. The hint is handled while building the logical 
plan, and the assertion only verifies the session variable change in the 
current `ConnectContext`.
   
   So the current test path does not depend on `test_t1` being resolved as a 
real table, and removing the OLAP table fixture does not affect this case for 
now. If the test later goes beyond pure parsing, we can add the fixture back.



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