Expose partition, file format and compression info of input tables as part of QueryCompletedEvent

[gaurav8297]

Description

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)

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

Expose the following things as part of QueryCompletedEvent which can be later tracked through telemetry.

• Whether the table partitioned?
• What kind of compression input tables are using?
• File format - Parquet/Orc

Related issues, pull requests, and links

Documentation

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

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

# Section
* Fix some things. ({issue}`issuenumber`)

[raunaqmorarka]

Please add some tests as well

[raunaqmorarka]

This shouldn't be optional, table will always have a file format

[raunaqmorarka]

Please confirm if the call to metastore.getTable is always satisfied by the transaction level metadata cache. Ideally, this shouldn't result in another metadata call to the underlying metastore.

[raunaqmorarka]

@alexjo2144 for iceberg and delta changes

[findepi]

Partitioned table may have multiple file formats (each partition can be different)

[raunaqmorarka]

Right, however I'm worried that that information may not be cheap to obtain. For our use case we're okay with just knowing about the current storage format configured for the table.
Would it be okay if we renamed fileFormat to tableFileFormat to be more explicit avoid the effort getting this info for all partitions ?

[findepi]

Would it be okay if we renamed fileFormat to tableFileFormat to be more explicit avoid the effort getting this info for all partitions ?

Seems like an OK workaround. Add code comment that's it's only as much useful for partitioned tables.

or call it tableDefaultFileFormat

[martint]

If this is the format of the table and not necessarily the format encountered by the query, it’s an odd thing to include in the query event. You can just obtain that data by looking at table metadata with SHOW CREATE TABLE, etc.

[raunaqmorarka]

@martint we want to be able to do an offline analysis about the relative occurrence of each file format for reads. We're recording the default table format instead of extracting the storage format of every partition to avoid adding overheads for collecting this info and because knowing the table level format is good enough approximation for telemetry use cases.

[findepi]

We have metics names, but we don't have metrics values.
What is the value supposed to mean?

[gaurav8297]

I've updated the value to be the data length as per this comment: #12551 (comment)

[findepi]

In Iceberg, can different files be of different format?

[gaurav8297]

Yes, it is possible that's why updated it to tableDefaultFileFormat.

[martint]

Please, move the change to add connector metrics support to the query event to a separate commit.

[raunaqmorarka]

If we're doing this only for testing with mock connector, can we keep the changes in MockConnectorPageSource ? You can merge the mock and delegate page source metrics in getMetrics

[raunaqmorarka]

Instead of sending dummy metrics via MockConnectorFactory, you can consider defining some actual metric like rowCount in MockConnectorPageSource if we get non-empty pages there during tests.

[raunaqmorarka]

lgtm % minor comments

[sopel39]

approved, planning to land it tomorrow if there are not objections from @martint or @findepi

[sopel39]

I don't think this needs release notes