PARQUET-484: Warn when Decimal is stored as INT64 while could be stored as INT32

[lw-lin]

Below is documented in LogicalTypes.md:

int32: for 1 <= precision <= 9
int64: for 1 <= precision <= 18; precision < 10 will produce a warning

This PR implements the precision < 10 will produce a warning part.

@rdblue @liancheng would mind taking a look at this when you have time? It's a fairly small addition; cheers.

[liancheng]

Had once noticed this issue but decided not to have this warning because this warning is mostly for Parquet data model developers. With this patch, when an end user uses a Parquet data model that doesn't always use the optimal primitive type for decimal, he/she may always see this warning, but can do nothing about it without updating the data model itself. This makes this warning quite disturbing and not helpful. Just my two cents.

[lw-lin]

@liancheng Thank your for the explanation, which is also reasonable I believe.
Is there some consensus of warning nor not reached so far in this community (I'm not aware of any)? I think it'd be good to make some changes either to the code or to the documentation, but this is not an urgent one after all.

[rdblue]

I think this warning is valuable. In the case of Spark or Hive, it is for data model developers, but it would definitely help fix a problem introduced there by flagging that the underlying data isn't using an efficient representation. In the case of object models like Avro, the user has control over the underlying type and would benefit from knowing if they chose too wide of a type.

[rdblue]

This should be static final Logger LOG and I would normally add an import for LoggerFactory.

[lw-lin]

Updated according to @rdblue 's code comments.

[lw-lin]

One thing worthy noticing is, we'll warn when precision <= 9 rather than when precision <= 10. This has been brought up in parquet-format PR #26.

[rdblue]

@lw-lin I think the logic here should match what was decided for the format (that PR has been merged). Is it the same in this PR?

[lw-lin]

Yes, it is the same here.

What was decided in the merged PR:

precision < 10 will produce a warning

What's implemented in this PR:

// MAX_PRECISION_INT32 is actually 9
if (meta.getPrecision() <= MAX_PRECISION_INT32) {
   warn...
}

@rdblue thanks for the revisit! :-)

[lw-lin]

@rdblue @liancheng thank you all for the review & merging :-)