infra-cluster: consul/helm/doris CWE-407 scan; buildkit/containerd/traefik/grafana/victoria-metrics/nifi/samza CLEAN
consul-0001: makeMeshGatewayPeerFilterChain peerNames O(S×B×P) slice scan → map[string]struct{}
helm-0001: ResourceList.Difference/Intersect O(N²) via Contains → pre-built map index
doris-0001: EquivalenceClass.getEquivalenceSetList ArrayList visited O(V²) → IdentityHashMap
doris-0002: PlanNode.addConjunct ArrayList.contains O(C²) → LinkedHashSet
6 CLEAN: buildkit, containerd, traefik, grafana, victoria-metrics, nifi (except pre-existing 0001), samza
This commit is contained in:
parent
cfed6f1c5b
commit
5b5355cb07
11 changed files with 479 additions and 50 deletions
|
|
@ -0,0 +1,78 @@
|
|||
# UNDF: UNDF-2026-000000381
|
||||
# UNDF: (pending)
|
||||
# doris-0001: EquivalenceClass.getEquivalenceSetList — O(N²) List.contains dedup
|
||||
|
||||
## CWE-407 — Algorithmic Complexity
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | doris-0001 |
|
||||
| Severity | MEDIUM |
|
||||
| Ecosystem | doris |
|
||||
| Package | fe-core |
|
||||
| File | `fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/EquivalenceClass.java` |
|
||||
| Lines | 130–146 |
|
||||
| Complexity | O(N²) |
|
||||
| Hot path | Called during materialized view rewrite in query planning (every query with MV candidates) |
|
||||
|
||||
## Defect
|
||||
|
||||
`getEquivalenceSetList()` deduplicates equivalence class lists using a `List<List<SlotReference>>`
|
||||
as the visited set. `visited.contains(slotSet)` calls `ArrayList.contains()` which iterates the
|
||||
entire visited list comparing lists element-by-element — O(N) per call. The outer loop also
|
||||
iterates N times, making the total complexity O(N²) where N is the number of distinct equivalence
|
||||
slot lists in `equivalenceSlotMap.values()`.
|
||||
|
||||
This runs on every query that touches materialized view rewrite logic in Doris Nereids.
|
||||
|
||||
```java
|
||||
public List<List<SlotReference>> getEquivalenceSetList() {
|
||||
if (equivalenceSlotList != null) {
|
||||
return equivalenceSlotList;
|
||||
}
|
||||
List<List<SlotReference>> equivalenceSets = new ArrayList<>();
|
||||
List<List<SlotReference>> visited = new ArrayList<>(); // O(N) per contains
|
||||
equivalenceSlotMap.values().forEach(slotSet -> {
|
||||
if (!visited.contains(slotSet)) { // O(N) scan
|
||||
equivalenceSets.add(slotSet);
|
||||
}
|
||||
visited.add(slotSet);
|
||||
});
|
||||
this.equivalenceSlotList = equivalenceSets;
|
||||
return this.equivalenceSlotList;
|
||||
}
|
||||
```
|
||||
|
||||
## Fix
|
||||
|
||||
`equivalenceSlotMap` maps each `SlotReference` to its equivalence-class list (which is a shared
|
||||
`List<SlotReference>` object). Deduplication by object identity is sufficient and correct because
|
||||
the union-find structure ensures all members of an equivalence class share the same list reference.
|
||||
Use `IdentityHashMap` or `Set` based on object identity:
|
||||
|
||||
```java
|
||||
public List<List<SlotReference>> getEquivalenceSetList() {
|
||||
if (equivalenceSlotList != null) {
|
||||
return equivalenceSlotList;
|
||||
}
|
||||
// Use identity comparison: all slots in the same equivalence class share the same List object
|
||||
Set<List<SlotReference>> seen = Collections.newSetFromMap(new IdentityHashMap<>());
|
||||
List<List<SlotReference>> equivalenceSets = new ArrayList<>();
|
||||
for (List<SlotReference> slotSet : equivalenceSlotMap.values()) {
|
||||
if (seen.add(slotSet)) { // O(1) identity hash
|
||||
equivalenceSets.add(slotSet);
|
||||
}
|
||||
}
|
||||
this.equivalenceSlotList = equivalenceSets;
|
||||
return this.equivalenceSlotList;
|
||||
}
|
||||
```
|
||||
|
||||
## Speedup
|
||||
|
||||
| N (equivalence classes) | Before (ops) | After (ops) | Speedup |
|
||||
|-------------------------|-------------|-------------|---------|
|
||||
| 10 | 100 | 10 | 10× |
|
||||
| 50 | 2,500 | 50 | 50× |
|
||||
| 100 | 10,000 | 100 | 100× |
|
||||
| 500 | 250,000 | 500 | 500× |
|
||||
|
|
@ -0,0 +1,101 @@
|
|||
# UNDF: (pending)
|
||||
# doris-0002: PlanNode.addConjunct — ArrayList.contains() O(C²) dedup
|
||||
|
||||
## CWE-407 — Algorithmic Complexity
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| ID | doris-0002 |
|
||||
| Severity | MEDIUM |
|
||||
| Ecosystem | doris |
|
||||
| Package | fe-core/planner |
|
||||
| File | `fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java` |
|
||||
| Lines | 308–324 |
|
||||
| Complexity | O(C²) — C conjuncts, each `addConjuncts()` call scans the list |
|
||||
| Hot path | Query planning — `addConjunctsToPlanNode` in PhysicalPlanTranslator |
|
||||
|
||||
## Defect
|
||||
|
||||
`PlanNode.conjuncts` is an `ArrayList<Expr>`. The `addConjunct` method deduplicates
|
||||
by calling `ArrayList.contains()`, which is O(C):
|
||||
|
||||
```java
|
||||
// PlanNode.java
|
||||
protected List<Expr> conjuncts = Lists.newArrayList();
|
||||
|
||||
public void addConjuncts(List<Expr> conjuncts) {
|
||||
if (conjuncts == null) return;
|
||||
for (Expr conjunct : conjuncts) { // O(C_new)
|
||||
addConjunct(conjunct);
|
||||
}
|
||||
}
|
||||
|
||||
public void addConjunct(Expr conjunct) {
|
||||
if (conjuncts == null) {
|
||||
conjuncts = Lists.newArrayList();
|
||||
}
|
||||
if (!conjuncts.contains(conjunct)) { // O(C_existing) scan
|
||||
conjuncts.add(conjunct);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`PhysicalPlanTranslator.addConjunctsToPlanNode` calls this from a double loop:
|
||||
|
||||
```java
|
||||
// PhysicalPlanTranslator.java:3039-3047
|
||||
private void addConjunctsToPlanNode(PhysicalFilter<? extends Plan> filter,
|
||||
PlanNode planNode, PlanTranslatorContext context) {
|
||||
for (Expression conjunct : filter.getConjuncts()) { // O(F)
|
||||
for (Expression singleConjunct : ExpressionUtils.extractConjunctionToSet(conjunct)) { // O(K)
|
||||
planNode.addConjunct(ExpressionTranslator.translate(singleConjunct, context)); // O(C)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Total: O(F × K × C) — with F=filter conjuncts, K=sub-conjuncts per predicate,
|
||||
C=existing conjuncts in node. At C=100 conjuncts, each `addConjunct` call costs
|
||||
100 equality comparisons instead of O(1).
|
||||
|
||||
## Fix
|
||||
|
||||
Replace `conjuncts` with a `LinkedHashSet<Expr>` (preserves insertion order,
|
||||
O(1) contains):
|
||||
|
||||
```java
|
||||
// Before:
|
||||
protected List<Expr> conjuncts = Lists.newArrayList();
|
||||
|
||||
public void addConjunct(Expr conjunct) {
|
||||
if (conjuncts == null) {
|
||||
conjuncts = Lists.newArrayList();
|
||||
}
|
||||
if (!conjuncts.contains(conjunct)) {
|
||||
conjuncts.add(conjunct);
|
||||
}
|
||||
}
|
||||
|
||||
// After:
|
||||
protected Set<Expr> conjunctSet = new LinkedHashSet<>();
|
||||
protected List<Expr> conjuncts = null; // lazy view, computed on demand
|
||||
|
||||
public void addConjunct(Expr conjunct) {
|
||||
conjunctSet.add(conjunct); // O(1) via hashCode/equals
|
||||
}
|
||||
|
||||
public List<Expr> getConjuncts() {
|
||||
return new ArrayList<>(conjunctSet);
|
||||
}
|
||||
```
|
||||
|
||||
If `Expr` does not implement `hashCode`/`equals`, a `LinkedHashMap<Expr, Boolean>`
|
||||
keyed by identity (`System.identityHashCode`) is the alternative.
|
||||
|
||||
## Speedup
|
||||
|
||||
| C (conjuncts) | addConjuncts calls | Before (ops) | After (ops) | Speedup |
|
||||
|---|---|---|---|---|
|
||||
| 20 | 20 | 400 | 20 | 20× |
|
||||
| 100 | 100 | 10,000 | 100 | 100× |
|
||||
| 500 | 500 | 250,000 | 500 | 500× |
|
||||
Loading…
Add table
Add a link
Reference in a new issue