PHOENIX-2288 Phoenix-Spark: PDecimal precision and scale aren't carried through to Spark DataFrame

[navis]

from jira description

When loading a Spark dataframe from a Phoenix table with a 'DECIMAL' type, the underlying precision and scale aren't carried forward to Spark.

The Spark catalyst schema converter should load these from the underlying column. These appear to be exposed in the ResultSetMetaData, but if there was a way to expose these somehow through ColumnInfo, it would be cleaner.

I'm not sure if Pig has the same issues or not, but I suspect it may.

It seemed enough just for current usage in spark-interagation. But in long term, PDataType should contain meta information like maxLength or precision, etc.

[jmahonin]

This looks great @navis

The Spark portion looks fine. I'll leave the updates to ColumnInfo for @ravimagham @JamesRTaylor et. al. to review

[JamesRTaylor]

Rather than check for particular types, it'd be more general to check for null like this:

Integer maxLength = pColumn.getMaxLength();
Integer scale = pColumn.getScale();
return new ColumnInfo(pColumn.toString(), sqlType, maxLength, scale);

Then make sure that ColumnInfo handles a null maxLength and scale.

[navis]

@JamesRTaylor How about to move the logic above into PColumn? Then we can access full information in it.

[JamesRTaylor]

ColumnInfo is a kind of lightweight transport class solely for passing in the necessary column metadata for the MR and Spark integration to run. It's passed in through the config so it has some simple to/from string methods - this prevents us from having to lookup the metadata from Phoenix metadata using the regular JDBC metadata APIs (which would be another option). Having this ColumnInfo class was deemed slightly easier.

PColumn has more information than we need and it'd be best to keep this as an internal/private class as much as possible. It's the object representation of our column metadata.

[JamesRTaylor]

Thanks for the pull request, @navis. A couple of minor comments, but overall it looks great. FYI, our PDataType class is stateless (it was an enum originally), so we currently access maxLength/precision and scale through the PDatum interface (from which PColumn and Expression are derived). Now that PDataType is no longer an enum, it might be nice to allow instantiation with maxLength and scale provided at construction time. Please file a JIRA.

[jmahonin]

How's this look, @JamesRTaylor / @ravimagham ?

[stoty]

Already merged.