Skip to content

Commit 08435a4

Browse files
yangzhgyangzhg
committed
[FIX] fix analyzer error in window function(apache#2039)
1 parent 5169eef commit 08435a4

File tree

3 files changed

+108
-133
lines changed

3 files changed

+108
-133
lines changed

‎fe/src/main/java/org/apache/doris/analysis/Analyzer.java

-52
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.List;
2929
import java.util.Map;
3030
import java.util.Set;
31-
import java.util.stream.Collectors;
3231

3332
import org.apache.doris.catalog.Catalog;
3433
import org.apache.doris.catalog.Column;
@@ -66,8 +65,6 @@
6665
import com.google.common.collect.Multimap;
6766
import com.google.common.collect.Sets;
6867

69-
//import org.apache.doris.catalog.InlineView;
70-
7168
/**
7269
* Repository of analysis state for single select block.
7370
* <p/>
@@ -285,9 +282,6 @@ public GlobalState(Catalog catalog, ConnectContext context) {
285282
// conjunct evaluating to false.
286283
private boolean hasEmptySpjResultSet_ = false;
287284

288-
private TupleDescriptor groupingVirtualTuple = null;
289-
private List<VirtualSlotRef> groupingSlots = new ArrayList<>();
290-
291285
public Analyzer(Catalog catalog, ConnectContext context) {
292286
ancestors = Lists.newArrayList();
293287
globalState = new GlobalState(catalog, context);
@@ -1745,50 +1739,4 @@ public void markRefdSlots(Analyzer analyzer, PlanNode planRoot,
17451739
}
17461740
}
17471741
}
1748-
1749-
1750-
public TupleDescriptor getGroupingVirtualTuple() {
1751-
return groupingVirtualTuple;
1752-
}
1753-
1754-
public void setGroupingVirtualTuple(TupleDescriptor groupingVirtualTuple) {
1755-
this.groupingVirtualTuple = groupingVirtualTuple;
1756-
}
1757-
1758-
public List<VirtualSlotRef> getGroupingSlots() {
1759-
return groupingSlots;
1760-
}
1761-
1762-
public VirtualSlotRef addGroupingSlots(List<SlotRef> realSlots) throws AnalysisException {
1763-
if (groupingVirtualTuple == null) {
1764-
groupingVirtualTuple = getDescTbl().createTupleDescriptor("VIRTUAL_TUPLE");
1765-
}
1766-
String colName = realSlots.stream().map(SlotRef::getColumnName).collect(Collectors.joining("_"));
1767-
colName = "GROUPING_PREFIX_" + colName;
1768-
VirtualSlotRef virtualSlot = new VirtualSlotRef(colName, Type.BIGINT, groupingVirtualTuple, realSlots);
1769-
virtualSlot.analyze(this);
1770-
int idx = groupingSlots.indexOf(virtualSlot);
1771-
if (idx > 0) {
1772-
return groupingSlots.get(idx);
1773-
}
1774-
groupingSlots.add(virtualSlot);
1775-
return virtualSlot;
1776-
}
1777-
public VirtualSlotRef addGroupingSlots(VirtualSlotRef virtualSlot) throws AnalysisException {
1778-
if (groupingVirtualTuple == null) {
1779-
groupingVirtualTuple = getDescTbl().createTupleDescriptor("VIRTUAL_TUPLE");
1780-
}
1781-
virtualSlot.analyze(this);
1782-
int idx = groupingSlots.indexOf(virtualSlot);
1783-
if (idx > 0) {
1784-
return groupingSlots.get(idx);
1785-
}
1786-
groupingSlots.add(virtualSlot);
1787-
return virtualSlot;
1788-
}
1789-
1790-
public void resetGrouping() {
1791-
groupingSlots = new ArrayList<>();
1792-
groupingVirtualTuple = null;
1793-
}
17941742
}

‎fe/src/main/java/org/apache/doris/analysis/GroupByClause.java

+83-67
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@
1919

2020
import java.util.ArrayList;
2121
import java.util.BitSet;
22+
import java.util.HashSet;
2223
import java.util.LinkedHashSet;
2324
import java.util.List;
2425
import java.util.Set;
26+
import java.util.stream.Collectors;
2527

28+
import org.apache.commons.collections.CollectionUtils;
29+
import org.apache.doris.catalog.Type;
2630
import org.apache.doris.common.AnalysisException;
2731
import org.apache.logging.log4j.LogManager;
2832
import org.apache.logging.log4j.Logger;
@@ -47,10 +51,10 @@ public class GroupByClause implements ParseNode {
4751
private GroupingType groupingType;
4852
private SyntaxType syntaxType;
4953
private ArrayList<Expr> groupingExprs;
50-
private ArrayList<Expr> groupingExprsForPrint;
54+
private ArrayList<Expr> oriGroupingExprs;
5155
private List<BitSet> groupingIdList;
5256
// grouping functions virtual slot
53-
private List<SlotRef> groupingSlots;
57+
private Set<VirtualSlotRef> groupingSlots;
5458
// reserve this info for toSQL
5559
private List<ArrayList<Expr>> groupingSetList;
5660
private ArrayList<Expr> groupByExprs;
@@ -64,24 +68,26 @@ public GroupByClause(List<ArrayList<Expr>> groupingSetList, ArrayList<Expr> grou
6468
this.groupingIdList = new ArrayList<>();
6569
this.groupByExprs = groupByExprs;
6670
Preconditions.checkState(type == GroupingType.GROUPING_SETS);
67-
groupingExprsForPrint = null;
68-
groupingSlots = new ArrayList<>();
71+
groupingSlots = new HashSet<>();
6972
virtualTuple = null;
7073
}
7174

7275
public GroupByClause(ArrayList<Expr> groupingExprs, GroupingType type, SyntaxType syntaxType) {
7376
this.groupingType = type;
7477
this.syntaxType = syntaxType;
75-
this.groupingExprs = groupingExprs;
78+
this.oriGroupingExprs = groupingExprs;
79+
this.groupingExprs = new ArrayList<>();
80+
this.groupingExprs.addAll(oriGroupingExprs);
7681
this.groupingIdList = new ArrayList<>();
7782
Preconditions.checkState(type != GroupingType.GROUPING_SETS);
78-
groupingExprsForPrint = Expr.cloneList(groupingExprs);
79-
groupingSlots = new ArrayList<>();
83+
groupingSlots = new HashSet<>();
8084
virtualTuple = null;
8185
}
8286
protected GroupByClause(GroupByClause other) {
8387
this.groupingType = other.groupingType;
8488
this.groupingExprs = (other.groupingExprs != null) ? Expr.cloneAndResetList(other.groupingExprs) : null;
89+
this.oriGroupingExprs =
90+
(other.oriGroupingExprs != null) ? Expr.cloneAndResetList(other.oriGroupingExprs) : null;
8591
if (other.groupingIdList != null) {
8692
this.groupingIdList = new ArrayList<>();
8793
for (BitSet bitSet : other.groupingIdList) {
@@ -97,67 +103,75 @@ protected GroupByClause(GroupByClause other) {
97103
this.groupingSetList.add(Expr.cloneAndResetList(exprList));
98104
}
99105
}
100-
if (other.groupingExprsForPrint != null) {
101-
this.groupingExprsForPrint = Expr.cloneAndResetList(other.groupingExprsForPrint);
102-
}
103106
if (other.groupingSlots != null) {
104-
this.groupingSlots = Expr.cloneList(other.groupingSlots);
107+
this.groupingSlots = new HashSet<>(Expr.cloneList(new ArrayList(other.groupingSlots)));
105108
}
106109
this.virtualTuple = other.virtualTuple;
107110
}
108111

109-
public void setVirtualTuple(TupleDescriptor virtualTuple) {
110-
this.virtualTuple = virtualTuple;
111-
}
112-
113-
public List<SlotRef> getGroupingSlots() {
114-
return groupingSlots;
112+
public void reset() {
113+
groupingExprs = new ArrayList<>();
114+
analyzed_ = false;
115+
exprGenerated = false;
116+
if (oriGroupingExprs != null) {
117+
Expr.resetList(oriGroupingExprs);
118+
groupingExprs.addAll(oriGroupingExprs);
119+
}
120+
groupingIdList = new ArrayList<>();
121+
groupingSlots = new HashSet<>();
122+
if (groupingSetList != null) {
123+
for (List<Expr> s : groupingSetList) {
124+
for (Expr e : s) {
125+
if (e != null) {
126+
e.reset();
127+
}
128+
}
129+
}
130+
}
131+
virtualTuple = null;
115132
}
116133

117134
public ArrayList<Expr> getGroupingExprs() {
118135
Preconditions.checkState(exprGenerated);
119136
return groupingExprs;
120137
}
121138

122-
public TupleId getGroupingTupleId() {
123-
if (!isGroupByExtension() || virtualTuple == null) {
139+
public TupleDescriptor getGroupingTuple() {
140+
if (!isGroupByExtension()) {
124141
return null;
125142
}
126-
return virtualTuple.getId();
143+
return virtualTuple;
127144
}
128145

129-
public TupleDescriptor getGroupingTuple() {
130-
if (!isGroupByExtension() || virtualTuple == null) {
146+
public TupleDescriptor getVirtualTuple(Analyzer analyzer) {
147+
if (!isGroupByExtension()) {
131148
return null;
149+
} else if (virtualTuple == null) {
150+
virtualTuple = analyzer.getDescTbl().createTupleDescriptor(VIRTUAL_TUPLE);
132151
}
133152
return virtualTuple;
134153
}
135154

136-
public void genVirtualTuple(Analyzer analyzer) {
137-
virtualTuple = analyzer.getDescTbl().createTupleDescriptor(VIRTUAL_TUPLE);
138-
analyzer.setGroupingVirtualTuple(virtualTuple);
139-
}
140-
141155
public void genGroupingExprs(Analyzer analyzer) throws AnalysisException {
142156
if (exprGenerated) {
143157
return;
144158
}
145-
if (groupingExprs != null && !groupingExprs.isEmpty()) {
159+
if (CollectionUtils.isNotEmpty(groupingExprs)) {
146160
// remove repeated element
147161
Set<Expr> groupingExprSet = new LinkedHashSet<>(groupingExprs);
148162
groupingExprs.clear();
149163
groupingExprs.addAll(groupingExprSet);
150164
}
151165
if (groupingType == GroupingType.CUBE || groupingType == GroupingType.ROLLUP) {
152-
if (groupingExprs == null || groupingExprs.isEmpty()) {
166+
if (CollectionUtils.isEmpty(groupingExprs)) {
153167
throw new AnalysisException(
154168
"The expresions in GROUPING CUBE or ROLLUP can not be empty");
155169
}
156170

157171
buildGroupingClause(analyzer);
158172

159173
} else if (groupingType == GroupingType.GROUPING_SETS) {
160-
if (groupingSetList == null || groupingSetList.isEmpty()) {
174+
if (CollectionUtils.isEmpty(groupingSetList)) {
161175
throw new AnalysisException("The expresions in GROUPINGING SETS can not be empty");
162176
}
163177
// collect all Expr elements
@@ -230,25 +244,23 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
230244
}
231245

232246
public boolean isGroupByExtension() {
233-
if (groupingType != GroupingType.GROUPING_SETS
234-
&& groupingType != GroupingType.CUBE
235-
&& groupingType != GroupingType.ROLLUP) {
247+
if (groupingType == GroupingType.GROUP_BY ||
248+
groupingType == GroupingType.GROUPING_SETS && (groupingSetList == null || groupingSetList.size() < 2)) {
236249
return false;
250+
} else {
251+
return true;
237252
}
238-
239-
return groupingType != GroupingType.GROUPING_SETS ||
240-
groupingSetList == null || groupingSetList.size() > 1;
241253
}
242254

243255
@Override
244256
public String toSql() {
245257
StringBuilder strBuilder = new StringBuilder();
246258
switch (groupingType) {
247259
case GROUP_BY:
248-
if (groupingExprsForPrint != null) {
249-
for (int i = 0; i < groupingExprsForPrint.size(); ++i) {
250-
strBuilder.append(groupingExprsForPrint.get(i).toSql());
251-
strBuilder.append((i + 1 != groupingExprsForPrint.size()) ? ", " : "");
260+
if (oriGroupingExprs != null) {
261+
for (int i = 0; i < oriGroupingExprs.size(); ++i) {
262+
strBuilder.append(oriGroupingExprs.get(i).toSql());
263+
strBuilder.append((i + 1 != oriGroupingExprs.size()) ? ", " : "");
252264
}
253265
}
254266
break;
@@ -279,32 +291,32 @@ public String toSql() {
279291
}
280292
break;
281293
case CUBE:
282-
if (groupingExprsForPrint != null && syntaxType == SyntaxType.ORACLE) {
294+
if (oriGroupingExprs != null && syntaxType == SyntaxType.ORACLE) {
283295
strBuilder.append("CUBE (");
284-
for (int i = 0; i < groupingExprsForPrint.size(); ++i) {
285-
strBuilder.append(groupingExprsForPrint.get(i).toSql());
286-
strBuilder.append((i + 1 != groupingExprsForPrint.size()) ? ", " : "");
296+
for (int i = 0; i < oriGroupingExprs.size(); ++i) {
297+
strBuilder.append(oriGroupingExprs.get(i).toSql());
298+
strBuilder.append((i + 1 != oriGroupingExprs.size()) ? ", " : "");
287299
}
288300
strBuilder.append(")");
289-
} else if (groupingExprsForPrint != null && syntaxType == SyntaxType.HIVE) {
290-
for (int i = 0; i < groupingExprsForPrint.size(); ++i) {
291-
strBuilder.append(groupingExprsForPrint.get(i).toSql());
292-
strBuilder.append((i + 1 != groupingExprsForPrint.size()) ? ", " : " WITH CUBE");
301+
} else if (oriGroupingExprs != null && syntaxType == SyntaxType.HIVE) {
302+
for (int i = 0; i < oriGroupingExprs.size(); ++i) {
303+
strBuilder.append(oriGroupingExprs.get(i).toSql());
304+
strBuilder.append((i + 1 != oriGroupingExprs.size()) ? ", " : " WITH CUBE");
293305
}
294306
}
295307
break;
296308
case ROLLUP:
297-
if (groupingExprsForPrint != null && syntaxType == SyntaxType.ORACLE) {
309+
if (oriGroupingExprs != null && syntaxType == SyntaxType.ORACLE) {
298310
strBuilder.append("ROLLUP (");
299-
for (int i = 0; i < groupingExprsForPrint.size(); ++i) {
300-
strBuilder.append(groupingExprsForPrint.get(i).toSql());
301-
strBuilder.append((i + 1 != groupingExprsForPrint.size()) ? ", " : "");
311+
for (int i = 0; i < oriGroupingExprs.size(); ++i) {
312+
strBuilder.append(oriGroupingExprs.get(i).toSql());
313+
strBuilder.append((i + 1 != oriGroupingExprs.size()) ? ", " : "");
302314
}
303315
strBuilder.append(")");
304-
} else if (groupingExprsForPrint != null && syntaxType == SyntaxType.HIVE) {
305-
for (int i = 0; i < groupingExprsForPrint.size(); ++i) {
306-
strBuilder.append(groupingExprsForPrint.get(i).toSql());
307-
strBuilder.append((i + 1 != groupingExprsForPrint.size()) ? ", " : " WITH ROLLUP");
316+
} else if (oriGroupingExprs != null && syntaxType == SyntaxType.HIVE) {
317+
for (int i = 0; i < oriGroupingExprs.size(); ++i) {
318+
strBuilder.append(oriGroupingExprs.get(i).toSql());
319+
strBuilder.append((i + 1 != oriGroupingExprs.size()) ? ", " : " WITH ROLLUP");
308320
}
309321
}
310322
break;
@@ -319,20 +331,24 @@ public GroupByClause clone() {
319331
return new GroupByClause(this);
320332
}
321333

322-
public void reset() {
323-
if (groupingExprs != null) {
324-
Expr.resetList(groupingExprs);
325-
}
326-
this.analyzed_ = false;
327-
}
328-
329334
public boolean isEmpty() {
330-
return groupingExprs == null || groupingExprs.isEmpty();
335+
return CollectionUtils.isEmpty(groupingExprs);
331336
}
332337

333-
public void addGroupingCol(List<VirtualSlotRef> vSlots) {
334-
groupingSlots.addAll(vSlots);
335-
groupingExprs.addAll(groupingSlots);
338+
public VirtualSlotRef addGroupingSlots(List<SlotRef> realSlots, Analyzer analyzer) throws AnalysisException {
339+
String colName = realSlots.stream().map(SlotRef::getColumnName).collect(Collectors.joining("_"));
340+
colName = "GROUPING_PREFIX_" + colName;
341+
VirtualSlotRef virtualSlot = new VirtualSlotRef(colName, Type.BIGINT, getVirtualTuple(analyzer), realSlots);
342+
virtualSlot.analyze(analyzer);
343+
if (groupingSlots.contains(virtualSlot)) {
344+
for (VirtualSlotRef vs : groupingSlots) {
345+
if (vs.equals(virtualSlot)) {
346+
return vs;
347+
}
348+
}
349+
}
350+
groupingSlots.add(virtualSlot);
351+
return virtualSlot;
336352
}
337353

338354
private void buildGroupingClause(Analyzer analyzer) {

0 commit comments

Comments
 (0)