Fix failure to read nullable time(6) columns

[joejensen]

Description

Fixes #15603

Additional context and related issues

Fixes issue where ORC files containing nullable Iceberg TIME columns could throw an exception when being read. Also fixed an error in the ORC file validator where time columns did not validate correctly in order to add appropriate tests for the Time type.

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Iceberg
* Fix Iceberg query failure when reading ORC files with nullable `time` columns. ({issue}`15606`)

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

Could you add a test in Iceberg connector as well?

[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

[joejensen]

Added relevant test in the Iceberg connector.

[joejensen]

Rebased onto current master.

[github-actions]

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

[bitsondatadev]

@raunaqmorarka are you able to review this?

[joejensen]

Rebasing to resolve merge conflict in TestIcebergOrcConnectorTest.

Anything I can do to help get this merged?

[bitsondatadev]

@joejensen looking into it now.

[ebyhr]

@joejensen Could you rebase on master to resolve conflicts?

[ebyhr]

Rebased on master to resolve conflicts and addressed comments except for #15606 (comment)

[raunaqmorarka]

This lgtm, but would be great to get another pair of eyes on it

[findepi]

Fixes #15603

@raunaqmorarka we should have connector-level test coverage for this.
Why TestIcebergMinioOrcConnectorTest.testDataMappingSmokeTest did not catch this for "time(6)" case?

[raunaqmorarka]

Fixes #15603

@raunaqmorarka we should have connector-level test coverage for this. Why TestIcebergMinioOrcConnectorTest.testDataMappingSmokeTest did not catch this for "time(6)" case?

That is because of a quirk in that test's data setup and the way orc reader works. There is only 1 null in the setup and that is in the first row. The orc reader reads in batches of 1, 2, 4 ... rows. So the first batch is all nulls, while the 2nd batch is all non-nulls. This bug requires a batch containing null as well as non-null values to cause failure.
I think improving that should be tackled separately from this PR, there is sufficient testing here to land this.