JackieTien97 commented on code in PR #17960:
URL: https://github.com/apache/iotdb/pull/17960#discussion_r3432746539


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java:
##########
@@ -1670,11 +2185,25 @@ private SelectAnalysis analyzeSelect(QuerySpecification 
node, Scope scope) {
               outputPosition++;
             }
           } else {
+            SelectAliasLookup visibleAliases = visibleAliasBuilder.build();

Review Comment:
   Nit / follow-up suggestion: this builds a fresh immutable 
`SelectAliasLookup` for every `SingleColumn`. Since the `SelectAliasLookup` 
constructor copies the whole current alias map and each alias list, analyzing a 
very wide SELECT list with many reusable aliases ends up doing repeated prefix 
copies, i.e. roughly O(N^2) alias-map copying.
   
   This is probably fine for normal SELECT lists, but it may be worth avoiding 
the repeated snapshots by using an incremental/shared immutable lookup 
structure, or by exposing a read-only lookup view over the builder whose 
visible state grows left-to-right.
   
   Could you also add IT/regression coverage for this area? In particular, it 
would be useful to cover a wide SELECT-list LCA chain/reuse case, plus the edge 
cases around delimited/quoted alias case sensitivity, named `WINDOW` 
definitions not seeing SELECT aliases, `DISTINCT` with LCA and no `ORDER BY`, 
and LCA references in window frame bounds.



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

Reply via email to