raminqaf commented on code in PR #29481:
URL: https://github.com/apache/flink/pull/29481#discussion_r4277782820
##########
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:
Good catch! Introduced a new index map and a check between the two index map
of the keys. Added the test to cover the edge case.
--
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]
