[GitHub] [flink] twalthr commented on a change in pull request #12130: [FLINK-17668][table] Fix shortcomings in new data structures

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

twalthr commented on a change in pull request #12130:
URL: https://github.com/apache/flink/pull/12130#discussion_r425056693



##########
File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/data/ArrayData.java
##########
@@ -215,4 +226,103 @@ static Object get(ArrayData array, int pos, LogicalType elementType) {
 				throw new UnsupportedOperationException("Unsupported type: " + elementType);
 		}
 	}
+
+	/**
+	 * Creates an accessor for getting elements in an internal array data structure at the
+	 * given position.
+	 *
+	 * @param elementType the element type of the array
+	 */
+	static ElementGetter createElementGetter(LogicalType elementType) {
+		final ElementGetter elementGetter;
+		// ordered by type root definition
+		switch (elementType.getTypeRoot()) {
+			case CHAR:
+			case VARCHAR:
+				elementGetter = ArrayData::getString;
+				break;
+			case BOOLEAN:
+				elementGetter = ArrayData::getBoolean;
+				break;
+			case BINARY:
+			case VARBINARY:
+				elementGetter = ArrayData::getBinary;
+				break;
+			case DECIMAL:
+				final int decimalPrecision = getPrecision(elementType);
+				final int decimalScale = getScale(elementType);
+				elementGetter = (array, pos) -> array.getDecimal(pos, decimalPrecision, decimalScale);
+				break;
+			case TINYINT:
+				elementGetter = ArrayData::getByte;
+				break;
+			case SMALLINT:
+				elementGetter = ArrayData::getShort;
+				break;
+			case INTEGER:
+			case DATE:
+			case TIME_WITHOUT_TIME_ZONE:
+			case INTERVAL_YEAR_MONTH:
+				elementGetter = ArrayData::getInt;
+				break;
+			case BIGINT:
+			case INTERVAL_DAY_TIME:
+				elementGetter = ArrayData::getLong;
+				break;
+			case FLOAT:
+				elementGetter = ArrayData::getFloat;
+				break;
+			case DOUBLE:
+				elementGetter = ArrayData::getDouble;
+				break;
+			case TIMESTAMP_WITHOUT_TIME_ZONE:
+			case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+				final int timestampPrecision = getPrecision(elementType);
+				elementGetter = (array, pos) -> array.getTimestamp(pos, timestampPrecision);
+				break;
+			case TIMESTAMP_WITH_TIME_ZONE:
+				throw new UnsupportedOperationException();
+			case ARRAY:
+				elementGetter = ArrayData::getArray;
+				break;
+			case MULTISET:
+			case MAP:
+				elementGetter = ArrayData::getMap;
+				break;
+			case ROW:
+			case STRUCTURED_TYPE:
+				final int rowFieldCount = getFieldCount(elementType);
+				elementGetter = (array, pos) -> array.getRow(pos, rowFieldCount);
+				break;
+			case DISTINCT_TYPE:
+				elementGetter = createElementGetter(((DistinctType) elementType).getSourceType());
+				break;
+			case RAW:
+				elementGetter = ArrayData::getRawValue;
+				break;
+			case NULL:
+			case SYMBOL:
+			case UNRESOLVED:
+			default:
+				throw new IllegalArgumentException();
+		}
+		if (!elementType.isNullable()) {
+			return elementGetter;

Review comment:
       A lot of locations ignore nullability. We should gradually fix this. By default a type is nullable, so the check is performed in most of the cases. The implementation of the method is correct (given its input parameters), if there is a problem it should be the callers task.




----------------------------------------------------------------
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