[Go][Parquet] pqarrow.FileWriter allow adding parquet metadata after writing rowgroups

[xxgreg]

Describe the enhancement requested

I'd like to add a key/value metadata field to the Parquet metadata. The value of the field is not known until after the row groups have been written.

It looks like it is possible to do this when using parquet/file.Writer but it isn't possible when using pqarrow.FileWriter.

Would it make sense to add some API to allow for this?

Or perhaps it's already there, and I can't see it.

Component(s)

Go

[xxgreg]

I added a quick change to allow this. See commits above.

I can add some tests and send a PR if there's any interest.

It turns out that although you can update the key value metadata on parquet/file.Writer, the changes don't get persisted, so I added a commit which changes this also.

[mapleFU]

Finish api LGTM.
cc @zeroshade

[zeroshade]

thanks @xxgreg!! There's definitely interest in this and thanks for fixing the bug with persisting the metadata. Please add some tests and send a PR, it should automatically add me as a codeowner to review it.

[xxgreg]

Apologies for the slow reply. And thanks for the super quick replies!

I've got a couple of other priorities to sort out first. I'm planning to pick this up again in 2 weeks.

[tschaub]

I'm also interested in being able to set the file metadata after writing row groups. The commit proposed by @xxgreg changes the signature of the metadata.Finish() method. From @mapleFU's comment above, it sounds like this API change would be acceptable. If this backwards incompatible change is acceptable, I would like to help write tests to get this in. Or alternatively, if a non-breaking change would be preferred, I could try to make that happen. Any suggestions from maintainers on the preferred direction (non-breaking change only or breaking change ok) would be appreciated.

[tschaub]

It would also be useful to see an example of how the writer.FileMetadata and writer.KeyValueMetadata fields are intended to be used.

I only see writer.FileMetadata being set and then immediately being used in the file_writer.go file. So it isn't clear why this field needs to be exported at all (but perhaps it is used by other packages) - or even why it is is a struct field.

The writer.KeyValueMetadata field is only set by the WithWriteMetadata() function and then only accessed by the NewParquetWriter() function. If it is only used for this purpose, it could be unexported.

I'm unclear on why the FileMetadata and KeyValueMetadata fields are exported on the file.Writer struct (but I'm probably missing other uses for both of these fields). Are these meant to settable before the writer is closed? Or should there be an explicit writer.SetKeyValueMetadata() method that would be called before closing a writer (somewhat similar to segmentio/parquet-go#399).

[zeroshade]

@tschaub I think it would be fine to create a backwards incompatible change here (as long as we mark the PR appropriately). While a non-breaking change would always be preferred (such as creating a new function instead of changing the existing one) it's not a deal breaker on this.

I'm unclear on why the FileMetadata and KeyValueMetadata fields are exported on the file.Writer struct (but I'm probably missing other uses for both of these fields). Are these meant to settable before the writer is closed? Or should there be an explicit writer.SetKeyValueMetadata() method that would be called before closing a writer (somewhat similar to segmentio/parquet-go#399).

You are exactly correct, they were meant to be settable before the writer is closed as Finish / Close is where the metadata gets written so they can be set / updated anytime before the writer is closed when it will actually write the metadata. I think it's reasonable to change these to be no longer exported and add a SetKeyValueMetadata method if everyone thinks this would be a better API.

[tschaub]

I've put together a proposed set of changes in #37786.