[C++][Parquet] I found formating errors in parquet.thrift

[OmBiradar]

Describe the enhancement requested

When I was using Zed code editor recently, and it's automatic formatter found some few extra spaces here and there in cpp/src/parquet/parquet.thrift. The main thing that bothers me is that these extra spaces are irregular - they don't follow any particular rule, that's why I think these need to be "fixed".

I get that this might be silly, but I am just putting it out there.

Checkout the the exact lines in my fork - main...OmBiradar:arrow:parquet.thrift-formatting-error

Component(s)

C++

Could you please look at it, @raulcd AND/OR anybody else who can provide feedback

[raulcd]

I am not a Parquet committer myself but this file should be a copy of the original file from the parquet-format repository:
https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift

The original copy also has those extra spaces. Using a formatter to remove extra spaces on the thrift file seems reasonable to me but this should be done in the source file and probably should be enforced through a linter.
@alamb @pitrou thoughts on this?

[alamb]

I agree -- cleaning up extra spaces on the parquet.thrift file seems very reasonable and I would be happy to review a PR

[OmBiradar]

@alamb just to confirm, you mean a PR to arrow right?

[alamb]

@alamb just to confirm, you mean a PR to arrow right?

Sorry I meant a PR to the parquet-format repo: https://github.com/apache/parquet-format

[OmBiradar]

Sorry I meant a PR to the parquet-format repo: https://github.com/apache/parquet-format

Sure!

One question I had is -

there are quite some differences between the parquet.thrift in the parquet-format repo (diff here) and the arrow repo, mostly it's just comments and a few lines of code.

How are the files synced?

[OmBiradar]

On the same note, would adding a linter to check the .thrift file linting be a good idea?

@alamb

[raulcd]

How are the files synced?

They are usually synced when required, a new feature that requires the updated thrift file is implemented on Parquet C++. We could update it once the new formatting file is merged upstream.

would adding a linter to check the .thrift file linting be a good idea?

Yes, I think it would be good to have the linter check as part of the PR that you submitted to the parquet-format repo.