Fix OPTIMIZE failing with uppercase Iceberg column names

[kinolaev]

Description

IcebergMetadata.getColumnHandles() stored data column handles with original-case keys from the Iceberg schema, but ColumnMetadata lowercases all names in its constructor. When getColumnHandlesForOptimize() (introduced in 480) looked up columns by their ColumnMetadata names, the case-insensitive lookup failed for tables with uppercase column names.

The fix lowercases the key in getColumnHandles() to match the convention already enforced by MetadataManager at the engine boundary, and to match what ColumnMetadata.getName() returns.

Release notes

## Iceberg
* Fix failure when column name contains uppercase characters in OPTIMIZE procedure. ({issue}`28888`)

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[ebyhr]

@kinolaev Thanks for detecting this bug quickly. Have you already submitted CLA?

[kinolaev]

Yes, around 16:30 utc

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[chenjian2664]

IMO, update the getColumnHandles isn't ideally, it expand the scope - since analyzer and planner depends on the method, though I am not very strong about it.

Instead can we consider update getColumnHandlesForOptimize to get the schema directly, sth like:

                for (IcebergColumnHandle columnHandle : getTopLevelColumns(SchemaParser.fromJson(table.getTableSchemaJson()), typeManager)) {
                    columnHandles.add(columnHandle);
                }

                if (supportsRowLineage(((IcebergTableHandle) tableHandle).getFormatVersion())) {
                    columnHandles
                            .add(rowIdColumnHandle())
                            .add(lastUpdatedSequenceNumberColumnColumnHandle());
                }

cc @ebyhr @dain @findepi

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[ebyhr]

Perhaps, the default implementation of SPI has the same problem:

trino/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java

Lines 150 to 164 in 0899de1

	default Set<ColumnHandle> getColumnHandlesForTableExecute(ConnectorSession connectorSession, ConnectorTableHandle tableHandle, ConnectorTableExecuteHandle connectorTableExecuteHandle)
	{
	Map<String, ColumnHandle> columnHandles = getColumnHandles(connectorSession, tableHandle);
	return getTableMetadata(connectorSession, tableHandle).getColumns().stream()
	.filter(not(ColumnMetadata::isHidden))
	.map(ColumnMetadata::getName)
	.map(columnName -> {
	ColumnHandle columnHandle = columnHandles.get(columnName);
	if (columnHandle == null) {
	throw new TrinoException(GENERIC_INTERNAL_ERROR, format("No handle found for column '%s' in table execute", columnName));
	}
	return columnHandle;
	})
	.collect(toUnmodifiableSet());
	}

# spark
CREATE TABLE test (X int) USING delta LOCATION 's3://test-bucket/test';
INSERT INTO test VALUES 1;
INSERT INTO test VALUES 2;

# trino
ALTER TABLE test EXECUTE optimize;
No handle found for column 'x' in table execute

[findepi]

Perhaps, the default implementation of SPI has the same problem

@ebyhr can you please file an issue?
if this is a recent regression, this might need to be a release blocker 🤔

[ebyhr]

@findepi Sure, filed #28970 with a release blocker label.

[ebyhr]

Let me continue the work at #28972 because it requires more work. I added @kinolaev as the co-author.