wuchong commented on a change in pull request #12130: URL: https://github.com/apache/flink/pull/12130#discussion_r425092603
########## 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: I still think we should be cautious here. As you said, most of the cases are nullable, so we don't get much performance improvement from this. However, it may occure unexpected bugs if it is called in planner. We can add a TODO on this. When we fix all the nullable problem, we can update this without breaking compatibility. > if there is a problem it should be the callers task. The problem is planner will also call this method, and I'm sure there is cases null values exist in NOT NULL fields, then an exception will happen. This will be a bug then. ---------------------------------------------------------------- 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: us...@infra.apache.org