MartijnVisser commented on code in PR #29481:
URL: https://github.com/apache/flink/pull/29481#discussion_r4275879157
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/MapFunctionITCase.java:
##########
@@ -889,4 +891,190 @@ private Stream<TestSetSpec> mapUnionTestCases() {
"Invalid input arguments. Expected signatures
are:\n"
+ "MAP_UNION(<COMMON>, <COMMON>...)"));
}
+
+ private Stream<TestSetSpec> mapEqualsTestCases() {
+ // In f1 the entries of f0 come in the opposite order, so the planner
cannot merge or fold
+ // the two sides. f2 differs from f0 in one value.
+ return Stream.of(
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.EQUALS,
"MAP<STRING, DOUBLE>")
+ .onFieldsWithData(
+ orderedMap(A, 1.5, B, 2.5),
+ orderedMap(B, 2.5, A, 1.5),
+ orderedMap(A, 1.5, B, 1.5))
+ .andDataTypes(
+ MAP(STRING(), DOUBLE()),
+ MAP(STRING(), DOUBLE()),
+ MAP(STRING(), DOUBLE()))
+ .testResult(
+ resultSpec($("f0").isEqual($("f1")), "f0 =
f1", true, BOOLEAN()),
+ resultSpec(
+ $("f0").isNotEqual($("f1")), "f0 <>
f1", false, BOOLEAN()),
+ resultSpec($("f0").isEqual($("f2")), "f0 =
f2", false, BOOLEAN()),
+ resultSpec(
+ $("f0").isNotEqual($("f2")), "f0 <>
f2", true, BOOLEAN())),
+ // 7 is a cached Integer box, 5000 is not
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.EQUALS,
"MAP<STRING, INT>")
+ .onFieldsWithData(
+ orderedMap(A, 7, B, 5000),
+ orderedMap(B, 5000, A, 7),
+ orderedMap(A, 7, B, 6000))
+ .andDataTypes(
+ MAP(STRING(), INT()), MAP(STRING(), INT()),
MAP(STRING(), INT()))
+ .testResult(
+ resultSpec($("f0").isEqual($("f1")), "f0 =
f1", true, BOOLEAN()),
+ resultSpec(
+ $("f0").isNotEqual($("f1")), "f0 <>
f1", false, BOOLEAN()),
+ resultSpec($("f0").isEqual($("f2")), "f0 =
f2", false, BOOLEAN()),
+ resultSpec(
+ $("f0").isNotEqual($("f2")), "f0 <>
f2", true, BOOLEAN())),
+ TestSetSpec.forFunction(BuiltInFunctionDefinitions.EQUALS,
"MAP<STRING, DATE>")
+ .onFieldsWithData(
+ orderedMap(A, TEST_DATE_1, B, TEST_DATE_2),
+ orderedMap(B, TEST_DATE_2, A, TEST_DATE_1),
+ orderedMap(A, TEST_DATE_1, B, TEST_DATE_1))
+ .andDataTypes(
+ MAP(STRING(), DATE()), MAP(STRING(), DATE()),
MAP(STRING(), DATE()))
+ .testResult(
+ resultSpec($("f0").isEqual($("f1")), "f0 =
f1", true, BOOLEAN()),
+ resultSpec(
+ $("f0").isNotEqual($("f1")), "f0 <>
f1", false, BOOLEAN()),
+ resultSpec($("f0").isEqual($("f2")), "f0 =
f2", false, BOOLEAN()),
+ resultSpec(
+ $("f0").isNotEqual($("f2")), "f0 <>
f2", true, BOOLEAN())),
+ // a reference type, so unboxing leaves the values as they are
Review Comment:
Nothing is unboxed anymore, so this comment can go.
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/ScalarOperatorGens.scala:
##########
@@ -1948,58 +1948,83 @@ object ScalarOperatorGens {
val rightTerm = args(1)
val resultTerm = newName(ctx, "compareResult")
+ val sizeTerm = newName(ctx, "size")
val mapCls = className[java.util.Map[_, _]]
- val keyCls = boxedTypeTermForType(keyType)
- val valueCls = boxedTypeTermForType(valueType)
+ val hashMapCls = className[java.util.HashMap[_, _]]
+ val valueCls = primitiveTypeTermForType(valueType)
+ val valueDefault = primitiveDefaultValue(valueType)
- val leftMapTerm = newName(ctx, "leftMap")
+ val keyGetterCls = classOf[ArrayData.ElementGetter].getCanonicalName
+ val keyGetterTerm =
+ ctx.addReusableObject(ArrayData.createElementGetter(keyType),
"keyGetter", keyGetterCls)
+
+ val leftKeysTerm = newName(ctx, "leftKeys")
+ val leftValuesTerm = newName(ctx, "leftValues")
+ val rightKeysTerm = newName(ctx, "rightKeys")
+ val rightValuesTerm = newName(ctx, "rightValues")
+ val rightKeyIndexTerm = newName(ctx, "rightKeyIndex")
+
+ val indexTerm = newName(ctx, "index")
+ val leftIndexTerm = newName(ctx, "leftIndex")
val leftKeyTerm = newName(ctx, "leftKey")
+ val rightIndexTerm = newName(ctx, "rightIndex")
+ val rightBoxedIndexTerm = newName(ctx, "rightBoxedIndex")
+
+ // The values are read in place from the value arrays. Reading them
from a java.util.Map
+ // would box them, and generateEquals compares boxes by reference.
val leftValueTerm = newName(ctx, "leftValue")
val leftValueNullTerm = newName(ctx, "leftValueIsNull")
- val leftValueExpr =
- GeneratedExpression(leftValueTerm, leftValueNullTerm, "", valueType)
+ val leftValueExpr = GeneratedExpression(leftValueTerm,
leftValueNullTerm, "", valueType)
- val rightMapTerm = newName(ctx, "rightMap")
val rightValueTerm = newName(ctx, "rightValue")
val rightValueNullTerm = newName(ctx, "rightValueIsNull")
- val rightValueExpr =
- GeneratedExpression(rightValueTerm, rightValueNullTerm, "",
valueType)
+ val rightValueExpr = GeneratedExpression(rightValueTerm,
rightValueNullTerm, "", valueType)
- val entryTerm = newName(ctx, "entry")
- val entryCls = classOf[java.util.Map.Entry[AnyRef,
AnyRef]].getCanonicalName
val valueEqualsExpr =
generateEquals(ctx, leftValueExpr, rightValueExpr, new
BooleanType(valueType.isNullable))
- val internalTypeCls = classOf[LogicalType].getCanonicalName
- val keyTypeTerm = ctx.addReusableObject(keyType, "keyType",
internalTypeCls)
- val valueTypeTerm = ctx.addReusableObject(valueType, "valueType",
internalTypeCls)
- val mapDataUtil = className[MapDataUtil]
-
val stmt =
s"""
|boolean $resultTerm;
- |if ($leftTerm.size() == $rightTerm.size()) {
+ |int $sizeTerm = $leftTerm.size();
+ |if ($sizeTerm == $rightTerm.size()) {
| $resultTerm = true;
- | $mapCls $leftMapTerm = $mapDataUtil
- | .convertToJavaMap($leftTerm, $keyTypeTerm, $valueTypeTerm);
- | $mapCls $rightMapTerm = $mapDataUtil
- | .convertToJavaMap($rightTerm, $keyTypeTerm,
$valueTypeTerm);
+ | $ARRAY_DATA $leftKeysTerm = $leftTerm.keyArray();
+ | $ARRAY_DATA $leftValuesTerm = $leftTerm.valueArray();
+ | $ARRAY_DATA $rightKeysTerm = $rightTerm.keyArray();
+ | $ARRAY_DATA $rightValuesTerm = $rightTerm.valueArray();
|
- | for ($entryCls $entryTerm : $leftMapTerm.entrySet()) {
- | $keyCls $leftKeyTerm = ($keyCls) $entryTerm.getKey();
- | if ($rightMapTerm.containsKey($leftKeyTerm)) {
- | $valueCls $leftValueTerm = ($valueCls)
$entryTerm.getValue();
- | $valueCls $rightValueTerm = ($valueCls)
$rightMapTerm.get($leftKeyTerm);
- | boolean $leftValueNullTerm = ($leftValueTerm == null);
- | boolean $rightValueNullTerm = ($rightValueTerm == null);
+ | $mapCls $rightKeyIndexTerm = new $hashMapCls();
+ | for (int $indexTerm = 0; $indexTerm < $sizeTerm; $indexTerm++)
{
+ | $rightKeyIndexTerm.put(
+ | $keyGetterTerm.getElementOrNull($rightKeysTerm,
$indexTerm),
+ | $indexTerm);
+ | }
|
- | ${valueEqualsExpr.code}
- | if (!${valueEqualsExpr.resultTerm}) {
- | $resultTerm = false;
- | break;
- | }
- | } else {
+ | for (int $leftIndexTerm = 0; $leftIndexTerm < $sizeTerm;
$leftIndexTerm++) {
+ | Object $leftKeyTerm =
$keyGetterTerm.getElementOrNull($leftKeysTerm, $leftIndexTerm);
+ | Integer $rightBoxedIndexTerm = (Integer)
$rightKeyIndexTerm.get($leftKeyTerm);
Review Comment:
Duplicate keys break this: `MAP[a, 1, b, 2] = MAP[c, 1, d, 2]` with all keys
'x' is TRUE on master, FALSE here. Can you also index the left keys, compare
the index sizes and add a test?
--
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]
