Avoid no-op Iceberg table snapshot if DML doesn't change table state

[findinpath]

Description

This PR avoids creating an Iceberg snapshot when dealing with a DML statement that does not affect the contents of the Iceberg table.

e.g. :

INSERT INTO iceberg.default.iceberg_table SELECT * FROM source_table WHERE false

NOTE however that a CTAS statement that corresponds to an empty SELECT will be obviously recorded as a table snapshot because it creates the table.

Is this change a fix, improvement, new feature, refactoring, or other?

Improvement

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Iceberg connector

How would you describe this change to a non-technical end user or system administrator?

Avoid creating a snapshot for a table when dealing with an empty DML (INSERT, UPDATE, DELETE) statement.

Related issues, pull requests, and links

Fixes #12319

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Iceberg
* Avoid creating a table snapshot for DML statements that do not change the table state

[electrum]

I think we could change this to

// avoid empty snapshot for existing tables
if (commitTasks.isEmpty() && (table.currentSnapshot() != null)) {

Then we don't need the special case in finishCreateTable().

[findinpath]

Yes, this could be done indeed as you mentioned.
This has been actually the initial version of the code in the PR.

However, the code would be a bit more difficult to understand with the handling for this special case in finishInsert so we've decided to go further with adding the handling for empty CTAS into finishCreateTable.

Related discussion:

#12412 (comment)

[homar]

Shouldn't this check happen before instantiating Type[] partitionColumnTypes ?

[findinpath]

I moved the check at the beginning of the method. Thanks for bringing this up.

[findepi]

@findinpath please update

@alexjo2144 please review after the update

[findepi]

why these two lines?

[findepi]

i see now, please add a comment

[findepi]

@findinpath do you want to suggest RN entry?

[findinpath]

do you want to suggest RN entry?

I think that the change in this PR is rather minor for Iceberg functionality and probably is not worth mentioning in the release notes.

I added however the following RN line the PR description.

Avoid creating a table snapshot for DML statements that do not change the table state

Up to you @findepi whether this should get included in the RN.