Skip to content

Commit e27fa6f

Browse files
committed
Add performance warning for MAP_FILTER
1 parent bc7c50b commit e27fa6f

File tree

3 files changed

+60
-1
lines changed

3 files changed

+60
-1
lines changed

presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/ExpressionAnalyzer.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.facebook.presto.common.type.MapType;
3131
import com.facebook.presto.common.type.RowType;
3232
import com.facebook.presto.common.type.Type;
33+
import com.facebook.presto.common.type.TypeSignature;
3334
import com.facebook.presto.common.type.TypeSignatureParameter;
3435
import com.facebook.presto.common.type.TypeUtils;
3536
import com.facebook.presto.common.type.TypeWithName;
@@ -154,6 +155,7 @@
154155
import static com.facebook.presto.common.type.VarcharType.VARCHAR;
155156
import static com.facebook.presto.metadata.BuiltInTypeAndFunctionNamespaceManager.JAVA_BUILTIN_NAMESPACE;
156157
import static com.facebook.presto.spi.StandardErrorCode.OPERATOR_NOT_FOUND;
158+
import static com.facebook.presto.spi.StandardWarningCode.PERFORMANCE_WARNING;
157159
import static com.facebook.presto.spi.StandardWarningCode.SEMANTIC_WARNING;
158160
import static com.facebook.presto.sql.NodeUtils.getSortItemsFromOrderBy;
159161
import static com.facebook.presto.sql.analyzer.Analyzer.verifyNoAggregateWindowOrGroupingFunctions;
@@ -1119,6 +1121,34 @@ else if (frame.getType() == GROUPS) {
11191121
}
11201122
}
11211123

1124+
List<TypeSignature> arguments = functionMetadata.getArgumentTypes();
1125+
String functionName = functionMetadata.getName().toString();
1126+
1127+
if (!argumentTypes.isEmpty() && "map".equals(arguments.get(0).getBase())) {
1128+
if (arguments.size() > 1) {
1129+
arguments.stream()
1130+
.skip(1)
1131+
.filter(arg -> {
1132+
String base = arg.getBase();
1133+
return "function".equals(base) || "lambda".equals(base);
1134+
})
1135+
.findFirst()
1136+
.ifPresent(arg -> {
1137+
String warningMessage = createWarningMessage(node,
1138+
String.format("Function '%s' uses a lambda on large maps which is expensive. Consider using map_subset", functionName));
1139+
warningCollector.add(new PrestoWarning(PERFORMANCE_WARNING, warningMessage));
1140+
}
1141+
);
1142+
} else if (arguments.size() == 1) {
1143+
String base = arguments.get(0).getBase();
1144+
if ("function".equals(base) || "lambda".equals(base)) {
1145+
String warningMessage = createWarningMessage(node,
1146+
String.format("Function '%s' uses a lambda on large maps which is expensive. Consider using map_subset", functionName));
1147+
warningCollector.add(new PrestoWarning(PERFORMANCE_WARNING, warningMessage));
1148+
}
1149+
}
1150+
}
1151+
11221152
if (node.isIgnoreNulls() && node.getWindow().isPresent()) {
11231153
if (!functionResolution.isWindowValueFunction(function)) {
11241154
String warningMessage = createWarningMessage(node, "IGNORE NULLS is not used for aggregate and ranking window functions. This will cause queries to fail in future versions.");

presto-main-base/src/test/java/com/facebook/presto/sql/analyzer/TestAnalyzer.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,35 @@ void testNoORWarning()
152152
assertNoWarning(analyzeWithWarnings("SELECT * FROM t1 JOIN t2 ON t1.a = t2.a \n" + "AND (t1.b = t2.b OR t1.b > t2.b)"));
153153
}
154154

155+
@Test
156+
public void testMapFilterWarnings()
157+
{
158+
assertHasWarning(
159+
analyzeWithWarnings("SELECT map_filter(x, (k, v) -> v > 1) FROM (VALUES (map(ARRAY[1,2], ARRAY[2,3]))) AS t(x)"),
160+
PERFORMANCE_WARNING,
161+
"Function 'presto.default.map_filter' uses a lambda on large maps which is expensive. Consider using map_subset");
162+
163+
assertHasWarning(
164+
analyzeWithWarnings("SELECT map_filter(x, (k, v) -> k = 2) FROM (VALUES (map(ARRAY[1,2,3], ARRAY[10,20,30]))) AS t(x)"),
165+
PERFORMANCE_WARNING,
166+
"Function 'presto.default.map_filter' uses a lambda on large maps which is expensive. Consider using map_subset");
167+
168+
assertHasWarning(
169+
analyzeWithWarnings("SELECT map_filter(x, (k, v) -> k IN (1, 3)) FROM (VALUES (map(ARRAY[1,2,3], ARRAY[10,20,30]))) AS t(x)"),
170+
PERFORMANCE_WARNING,
171+
"Function 'presto.default.map_filter' uses a lambda on large maps which is expensive. Consider using map_subset");
172+
173+
assertHasWarning(
174+
analyzeWithWarnings("SELECT map_filter(x, (k, v) -> v IN (20, 30)) FROM (VALUES (map(ARRAY[1,2,3], ARRAY[10,20,30]))) AS t(x)"),
175+
PERFORMANCE_WARNING,
176+
"Function 'presto.default.map_filter' uses a lambda on large maps which is expensive. Consider using map_subset");
177+
178+
assertHasWarning(
179+
analyzeWithWarnings("SELECT map_filter(x, (k, v) -> k + v > 25) FROM (VALUES (map(ARRAY[1,2,3], ARRAY[10,20,30]))) AS t(x)"),
180+
PERFORMANCE_WARNING,
181+
"Function 'presto.default.map_filter' uses a lambda on large maps which is expensive. Consider using map_subset");
182+
}
183+
155184
@Test
156185
public void testIgnoreNullWarning()
157186
{

presto-tests/src/test/java/com/facebook/presto/execution/TestWarnings.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ public void testMapWithDoubleKeysProducesWarnings()
180180
assertWarnings(queryRunner, TEST_SESSION, query, ImmutableSet.of(SEMANTIC_WARNING.toWarningCode()));
181181

182182
query = "select transform_keys(map(ARRAY [25.5E0, 26.5E0, 27.5E0], ARRAY [25.5E0, 26.5E0, 27.5E0]), (k, v) -> k + v)";
183-
assertWarnings(queryRunner, TEST_SESSION, query, ImmutableSet.of(SEMANTIC_WARNING.toWarningCode()));
183+
assertWarnings(queryRunner, TEST_SESSION, query, ImmutableSet.of(SEMANTIC_WARNING.toWarningCode(), PERFORMANCE_WARNING.toWarningCode()));
184184

185185
query = "SELECT histogram(RETAILPRICE) FROM tpch.tiny.part";
186186
assertWarnings(queryRunner, TEST_SESSION, query, ImmutableSet.of(SEMANTIC_WARNING.toWarningCode()));

0 commit comments

Comments
 (0)