[GitHub] [flink] dawidwys commented on a change in pull request #10158: [FLINK-14694] [table-planner-blink] Most tests from package o.a.f.table.planner.functions.aggfunctions are not executed during mvn test and fix BinaryGeneric comparison failure

[GitBox <gi...@apache.org>]

dawidwys commented on a change in pull request #10158: [FLINK-14694] [table-planner-blink] Most tests from package o.a.f.table.planner.functions.aggfunctions are not executed during mvn test and fix BinaryGeneric comparison failure
URL: https://github.com/apache/flink/pull/10158#discussion_r345136485
 
 

 ##########
 File path: flink-table/flink-table-planner-blink/src/test/java/org/apache/flink/table/planner/functions/aggfunctions/AggFunctionTestBase.java
 ##########
 @@ -224,6 +235,55 @@ public void testResetAccumulator() throws NoSuchMethodException, InvocationTarge
 			assertEquals(e.max, r.max);
 			assertEquals(e.mapSize, r.mapSize);
 		} else {
+			// verify null
+			if (expected == null || result == null) {
 
 Review comment:
   This code block can be significantly improved:
   
   ```
   	protected <E> void validateResult(E expected, E result, TypeInformation<?> typeInfo) {
   		if (expected instanceof BigDecimal && result instanceof BigDecimal) {
   			// BigDecimal.equals() value and scale but we are only interested in value.
   			assertEquals(0, ((BigDecimal) expected).compareTo((BigDecimal) result));
   		} else if (expected instanceof MinWithRetractAccumulator &&
   				result instanceof MinWithRetractAccumulator) {
   			MinWithRetractAccumulator e = (MinWithRetractAccumulator) expected;
   			MinWithRetractAccumulator r = (MinWithRetractAccumulator) result;
   			assertEquals(e.min, r.min);
   			assertEquals(e.mapSize, r.mapSize);
   		} else if (expected instanceof MaxWithRetractAccumulator &&
   				result instanceof MaxWithRetractAccumulator) {
   			MaxWithRetractAccumulator e = (MaxWithRetractAccumulator) expected;
   			MaxWithRetractAccumulator r = (MaxWithRetractAccumulator) result;
   			assertEquals(e.max, r.max);
   			assertEquals(e.mapSize, r.mapSize);
   		} else if (expected instanceof BinaryGeneric && result instanceof BinaryGeneric) {
   			TypeSerializer<?> serializer = typeInfo.createSerializer(new ExecutionConfig());
   			assertThat(
   				(BinaryGeneric) result,
   				equivalent((BinaryGeneric) expected, new BinaryGenericSerializer<>(serializer)));
   		} else if (expected instanceof GenericRow && result instanceof GenericRow) {
   			validateGenericRow(
   				(GenericRow) expected,
   				(GenericRow) result,
   				(BaseRowTypeInfo) typeInfo);
   		} else {
   			assertEquals(expected, result);
   		}
   	}
   
   	private void validateGenericRow(GenericRow expected, GenericRow result, BaseRowTypeInfo typeInfo) {
   		assertEquals(expected.getArity(), result.getArity());
   
   		for (int i = 0; i < expected.getArity(); ++i) {
   			Object expectedObj = expected.getField(i);
   			Object resultObj = result.getField(i);
   			validateResult(expectedObj, resultObj, typeInfo.getTypeAt(i));
   		}
   	}
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services