This is an automated email from the ASF dual-hosted git repository.
mihaibudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new b8cb1054fb [CALCITE-7780] Avoid redundant map lookups
b8cb1054fb is described below
commit b8cb1054fbba2b165b30b07ee1597015d9eff8fe
Author: Michael Braun <[email protected]>
AuthorDate: Mon Sep 14 23:36:35 2026 -0400
[CALCITE-7780] Avoid redundant map lookups
Several places look up the same map key two or three times in a row,
where one call to an existing Map method does the same job. For example,
HepPlanner#updateVertex removes a digest only if it still maps to the
vertex being updated, which is Map#remove(key, value).
The same shape occurs in a handful of other classes, and is replaced
with the corresponding single-lookup idiom: a get plus null check for
containsKey followed by get; putIfAbsent or computeIfAbsent for
containsKey followed by put; iteration over entrySet, with
Map.Entry#setValue for in-place updates, for iterating keySet and
calling get per key.
Some sites are on per-row paths (AggregateNode#send, UncollectNode and
the expression caches in RexToLixTranslator); the rest is planner and
validator code. No lambda introduced captures enclosing state, so the
computeIfAbsent call sites allocate nothing.
All values involved are non-null, so replacing containsKey with a null
check on get preserves behaviour. There is no functional change.
MongoFilter and QueryBuilders were reverted from this change: in
MongoFilter the map is declared Map<String, @Nullable Object>, and in
QueryBuilders the map is supplied by the caller, so in neither case is a
non-null value guaranteed by the type. Treating "absent" and "mapped to
null" alike is safe for the keys these two sites use, but it rests on an
invariant rather than on the declaration, so leave both as they were.
Strong#createPolicyMap is reverted for a different reason: the map is an
EnumMap, which does not override putIfAbsent, so the default Map
implementation still performs a get followed by a put. No lookup is
saved.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
---
.../adapter/enumerable/RexToLixTranslator.java | 25 ++++++++++++---------
.../apache/calcite/interpreter/AggregateNode.java | 7 +++---
.../apache/calcite/interpreter/UncollectNode.java | 8 +++----
.../java/org/apache/calcite/plan/RelOptUtil.java | 5 +++--
.../org/apache/calcite/plan/hep/HepPlanner.java | 4 +---
.../java/org/apache/calcite/rel/core/Match.java | 9 ++------
.../calcite/rel/externalize/RelDotWriter.java | 8 ++++---
.../rel/rules/CombineSimpleEquivalenceRule.java | 5 ++---
.../calcite/rel/rules/LoptOptimizeJoinRule.java | 5 +++--
.../main/java/org/apache/calcite/rex/RexUtil.java | 26 ++++++++--------------
.../calcite/sql/validate/SqlValidatorImpl.java | 3 +--
.../sql2rel/RelStructuredTypeFlattener.java | 17 +++++++-------
12 files changed, 58 insertions(+), 64 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
index 57ac8495c8..90437caa75 100644
---
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
+++
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
@@ -1454,8 +1454,9 @@ private static Expression scaleValue(
final Pair<RexNode, @Nullable Type> key = Pair.of(inputRef,
currentStorageType);
// If the RexInputRef has been visited under current storage type already,
// it is not necessary to visit it again, just return the result.
- if (rexWithStorageTypeResultMap.containsKey(key)) {
- return rexWithStorageTypeResultMap.get(key);
+ final Result cached = rexWithStorageTypeResultMap.get(key);
+ if (cached != null) {
+ return cached;
}
// Generate one line of code to get the input, e.g.,
// "final Employee current =(Employee) inputEnumerator.current();"
@@ -1519,8 +1520,9 @@ private static Expression scaleValue(
*/
@Override public Result visitLiteral(RexLiteral literal) {
// If the RexLiteral has been visited already, just return the result
- if (rexResultMap.containsKey(literal)) {
- return rexResultMap.get(literal);
+ final Result cached = rexResultMap.get(literal);
+ if (cached != null) {
+ return cached;
}
// Generate one line of code for the value of RexLiteral, e.g.,
// "final int literal_value = 10;"
@@ -1613,8 +1615,9 @@ private ConstantExpression getTypedNullLiteral(RexLiteral
literal) {
* need to be implemented separately.
*/
@Override public Result visitCall(RexCall call) {
- if (rexResultMap.containsKey(call)) {
- return rexResultMap.get(call);
+ final Result cached = rexResultMap.get(call);
+ if (cached != null) {
+ return cached;
}
final SqlOperator operator = call.getOperator();
if (operator == PREV) {
@@ -1812,8 +1815,9 @@ private Result toInnerStorageType(Result result, Type
storageType) {
@Override public Result visitDynamicParam(RexDynamicParam dynamicParam) {
final Pair<RexNode, @Nullable Type> key =
Pair.of(dynamicParam, currentStorageType);
- if (rexWithStorageTypeResultMap.containsKey(key)) {
- return rexWithStorageTypeResultMap.get(key);
+ final Result cached = rexWithStorageTypeResultMap.get(key);
+ if (cached != null) {
+ return cached;
}
final Type valueType = typeFactory.getJavaClass(dynamicParam.getType());
final Type storageType = currentStorageType != null ? currentStorageType :
valueType;
@@ -1857,8 +1861,9 @@ private Result toInnerStorageType(Result result, Type
storageType) {
@Override public Result visitFieldAccess(RexFieldAccess fieldAccess) {
final Pair<RexNode, @Nullable Type> key =
Pair.of(fieldAccess, currentStorageType);
- if (rexWithStorageTypeResultMap.containsKey(key)) {
- return rexWithStorageTypeResultMap.get(key);
+ final Result cached = rexWithStorageTypeResultMap.get(key);
+ if (cached != null) {
+ return cached;
}
final RexNode target = deref(fieldAccess.getReferenceExpr());
int fieldIndex = fieldAccess.getField().getIndex();
diff --git
a/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
b/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
index 684baad8d4..1e4a97040c 100644
--- a/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
+++ b/core/src/main/java/org/apache/calcite/interpreter/AggregateNode.java
@@ -382,15 +382,16 @@ public void send(Row row) {
}
Row key = builder.build();
- if (!accumulators.containsKey(key)) {
- AccumulatorList list = new AccumulatorList();
+ AccumulatorList list = accumulators.get(key);
+ if (list == null) {
+ list = new AccumulatorList();
for (AccumulatorFactory factory : accumulatorFactories) {
list.add(factory.get());
}
accumulators.put(key, list);
}
- accumulators.get(key).send(row);
+ list.send(row);
}
public void end(Sink sink) throws InterruptedException {
diff --git
a/core/src/main/java/org/apache/calcite/interpreter/UncollectNode.java
b/core/src/main/java/org/apache/calcite/interpreter/UncollectNode.java
index 725f577c4a..38372df630 100644
--- a/core/src/main/java/org/apache/calcite/interpreter/UncollectNode.java
+++ b/core/src/main/java/org/apache/calcite/interpreter/UncollectNode.java
@@ -61,16 +61,16 @@ public UncollectNode(Compiler compiler, Uncollect
uncollect) {
}
}
} else if (value instanceof Map) {
- Map map = (Map) value;
+ Map<?, ?> map = (Map<?, ?>) value;
if (map.isEmpty() && rel.isOuter) {
sink.send(Row.of(new Object[width]));
continue;
}
- for (Object key : map.keySet()) {
+ for (Map.Entry<?, ?> entry : map.entrySet()) {
if (rel.withOrdinality) {
- sink.send(Row.of(key, map.get(key), i++));
+ sink.send(Row.of(entry.getKey(), entry.getValue(), i++));
} else {
- sink.send(Row.of(key, map.get(key)));
+ sink.send(Row.of(entry.getKey(), entry.getValue()));
}
}
} else {
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 42e86b5c4b..d92a436595 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -2773,8 +2773,9 @@ public static void inferViewPredicates(Map<Integer,
RexNode> projectMap,
}
if (o0 instanceof RexInputRef && o1 instanceof RexLiteral) {
final int index = ((RexInputRef) o0).getIndex();
- if (projectMap.get(index) == null) {
- projectMap.put(index, o1);
+ // The first constraint on a column populates projectMap; a later
+ // constraint on the same column remains in filters.
+ if (projectMap.putIfAbsent(index, o1) == null) {
continue;
}
}
diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
index d2a39d4a43..6952fedb0a 100644
--- a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java
@@ -1080,9 +1080,7 @@ private void updateVertex(HepRelVertex vertex, RelNode
rel) {
notifyDiscard(vertex.getCurrentRel());
}
RelDigest oldKey = vertex.getCurrentRel().getRelDigest();
- if (mapDigestToVertex.get(oldKey) == vertex) {
- mapDigestToVertex.remove(oldKey);
- }
+ mapDigestToVertex.remove(oldKey, vertex);
// When a transformation happened in one rule apply, support
// vertex2 replace vertex1, but the current relNode of
// vertex1 and vertex2 is same,
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Match.java
b/core/src/main/java/org/apache/calcite/rel/core/Match.java
index a0d106b209..1fd61e7227 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Match.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Match.java
@@ -260,13 +260,8 @@ private static class AggregateFinder extends
RexVisitorImpl<Void> {
pv.add(STAR);
}
for (String alpha : pv) {
- final NavigableSet<RexMRAggCall> set;
- if (aggregateCallsPerVar.containsKey(alpha)) {
- set = aggregateCallsPerVar.get(alpha);
- } else {
- set = new TreeSet<>();
- aggregateCallsPerVar.put(alpha, set);
- }
+ final NavigableSet<RexMRAggCall> set =
+ aggregateCallsPerVar.computeIfAbsent(alpha, k -> new
TreeSet<>());
boolean update = true;
for (RexMRAggCall rex : set) {
if (rex.equals(aggCall)) {
diff --git
a/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java
b/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java
index f354bda246..46dee53efa 100644
--- a/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java
+++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java
@@ -31,6 +31,7 @@
import java.io.PrintWriter;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
@@ -187,9 +188,10 @@ private void explainInputs(List<? extends @Nullable
RelNode> inputs) {
pw.println("digraph {");
// print nodes with styles
- for (RelNode rel : nodeStyles.keySet()) {
- String style = String.join(",", nodeStyles.get(rel));
- pw.println(nodeLabels.get(rel) + " [style=\"" + style + "\"]");
+ for (Map.Entry<RelNode, Collection<String>> entry
+ : nodeStyles.asMap().entrySet()) {
+ String style = String.join(",", entry.getValue());
+ pw.println(nodeLabels.get(entry.getKey()) + " [style=\"" + style +
"\"]");
}
// ordinary arcs
diff --git
a/core/src/main/java/org/apache/calcite/rel/rules/CombineSimpleEquivalenceRule.java
b/core/src/main/java/org/apache/calcite/rel/rules/CombineSimpleEquivalenceRule.java
index eba15bcddc..86166c9639 100644
---
a/core/src/main/java/org/apache/calcite/rel/rules/CombineSimpleEquivalenceRule.java
+++
b/core/src/main/java/org/apache/calcite/rel/rules/CombineSimpleEquivalenceRule.java
@@ -170,9 +170,8 @@ private static RelHomogeneousShuttle getReplacer(
@Override public RelNode visit(RelNode node) {
// Check if this node's digest matches any of our shared components
RelDigest nodeDigest = node.getRelDigest();
- if (digestToSpool.containsKey(nodeDigest)) {
- LogicalTableSpool spool = digestToSpool.get(nodeDigest);
-
+ LogicalTableSpool spool = digestToSpool.get(nodeDigest);
+ if (spool != null) {
if (producers.contains(nodeDigest)) {
// Subsequent occurrence - replace with table scan (consumer)
return LogicalTableScan.create(
diff --git
a/core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java
b/core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java
index 16cb367ac2..44a7ff8309 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java
@@ -329,8 +329,9 @@ private static void findRemovableSelfJoins(RelMetadataQuery
mq, LoptMultiJoin mu
// From the candidate self-join pairs, determine if there is
// the appropriate join condition between the two factors that will
// allow the join to be removed.
- for (Integer factor1 : selfJoinPairs.keySet()) {
- final int factor2 = selfJoinPairs.get(factor1);
+ for (Map.Entry<Integer, Integer> selfJoinPair : selfJoinPairs.entrySet()) {
+ final int factor1 = selfJoinPair.getKey();
+ final int factor2 = selfJoinPair.getValue();
final List<RexNode> selfJoinFilters = new ArrayList<>();
for (RexNode filter : multiJoin.getJoinFilters()) {
ImmutableBitSet joinFactors =
diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
index 99045f0a3c..29b63409ad 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java
@@ -3197,14 +3197,12 @@ private void combinePredicatesUsingAnd(
Map<K, RexNode> baseMap,
Map<K, RexNode> forMergeMap) {
for (Map.Entry<K, RexNode> entry : forMergeMap.entrySet()) {
+ final RexNode baseRex = baseMap.get(entry.getKey());
RexNode mergedRex =
- relBuilder.and(
- entry.getValue(),
- baseMap.getOrDefault(entry.getKey(),
relBuilder.literal(true)));
+ relBuilder.and(entry.getValue(),
+ baseRex == null ? relBuilder.literal(true) : baseRex);
int originalCount = entry.getValue().nodeCount()
- + (baseMap.containsKey(entry.getKey())
- ? baseMap.get(entry.getKey()).nodeCount()
- : 0);
+ + (baseRex == null ? 0 : baseRex.nodeCount());
checkExpandCount(mergedRex.nodeCount() - originalCount);
baseMap.put(entry.getKey(), mergedRex);
}
@@ -3233,23 +3231,17 @@ private void combinePredicatesUsingOr(
Iterator<Map.Entry<K, RexNode>> iterator =
baseMap.entrySet().iterator();
while (iterator.hasNext()) {
- int forMergeNodeCount = 0;
-
Map.Entry<K, RexNode> entry = iterator.next();
- if (!forMergeMap.containsKey(entry.getKey())) {
+ final RexNode forMergeRex = forMergeMap.get(entry.getKey());
+ if (forMergeRex == null) {
checkExpandCount(-entry.getValue().nodeCount());
iterator.remove();
continue;
- } else {
- forMergeNodeCount = forMergeMap.get(entry.getKey()).nodeCount();
}
- RexNode mergedRex =
- relBuilder.or(
- entry.getValue(),
- forMergeMap.get(entry.getKey()));
- int originalCount = entry.getValue().nodeCount() + forMergeNodeCount;
+ RexNode mergedRex = relBuilder.or(entry.getValue(), forMergeRex);
+ int originalCount = entry.getValue().nodeCount() +
forMergeRex.nodeCount();
checkExpandCount(mergedRex.nodeCount() - originalCount);
- baseMap.put(entry.getKey(), mergedRex);
+ entry.setValue(mergedRex);
}
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 85a0f535a2..78c9d0d522 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -8793,8 +8793,7 @@ private SqlNode expandAliases(SqlIdentifier id,
CalciteContextException ex) {
if (expr instanceof SqlIdentifier) {
expr = getScope().fullyQualify((SqlIdentifier) expr).identifier;
}
- if (!expansions.containsKey(name)) {
- expansions.put(name, expr);
+ if (expansions.putIfAbsent(name, expr) == null) {
validator.setOriginal(expr, id);
}
return expr;
diff --git
a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
index ae095af32d..713aeb9be2 100644
---
a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
+++
b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
@@ -197,23 +197,24 @@ private RelNode getCurrentRelOrThrow() {
public void updateRelInMap(
SortedSetMultimap<RelNode, CorrelationId> mapRefRelToCorVar) {
for (RelNode rel : Lists.newArrayList(mapRefRelToCorVar.keySet())) {
- if (oldToNewRelMap.containsKey(rel)) {
+ RelNode newRel = oldToNewRelMap.get(rel);
+ if (newRel != null) {
SortedSet<CorrelationId> corVarSet =
mapRefRelToCorVar.removeAll(rel);
- mapRefRelToCorVar.putAll(oldToNewRelMap.get(rel), corVarSet);
+ mapRefRelToCorVar.putAll(newRel, corVarSet);
}
}
}
- @SuppressWarnings({"JdkObsolete", "ModifyCollectionInEnhancedForLoop"})
+ @SuppressWarnings("JdkObsolete")
public void updateRelInMap(
SortedMap<CorrelationId, LogicalCorrelate> mapCorVarToCorRel) {
- for (CorrelationId corVar : mapCorVarToCorRel.keySet()) {
- LogicalCorrelate oldRel = mapCorVarToCorRel.get(corVar);
- if (oldToNewRelMap.containsKey(oldRel)) {
- RelNode newRel = oldToNewRelMap.get(oldRel);
+ for (Map.Entry<CorrelationId, LogicalCorrelate> entry
+ : mapCorVarToCorRel.entrySet()) {
+ RelNode newRel = oldToNewRelMap.get(entry.getValue());
+ if (newRel != null) {
assert newRel instanceof LogicalCorrelate;
- mapCorVarToCorRel.put(corVar, (LogicalCorrelate) newRel);
+ entry.setValue((LogicalCorrelate) newRel);
}
}
}
