Allow for Unsigned Statistics in Binary Type

[asfimport]

BinaryStatistics currently only have a min/max, which are compared as signed byte[]. However, for real UTF8-friendly lexicographic comparison, e.g. for string columns, we would want to calculate the BinaryStatistics based off of a comparator that treats the bytes as unsigned.

Reporter: Andrew Duffy
Assignee: Ryan Blue / @rdblue

Related issues:

• Release Parquet format 2.4.0 (blocks)
• Release Parquet-mr 1.9.0 (blocks)
• Min-max should be computed based on logical type (is duplicated by)
• Statistics is not available for DECIMAL types (causes)
• Reference column_order field from column indexes (relates to)
• Parquet String Pushdown for Non-Eq Comparisons Broken (is related to)

PRs and other links:

• Format PR #42
• PR #362
• Format PR #46
• Format PR #55
• PR #367

_{Note: This issue was originally created as PARQUET-686. Please see the migration documentation for further details.}

[asfimport]

Piyush Narang / @piyushnarang:
[~andrew_duffy] do you have a link to the corresponding avro / spark code handy? That would make fixing this much easier.

[asfimport]

Piyush Narang / @piyushnarang:
Umm nevermind, just saw your PR :-)

[asfimport]

Cheng Lian / @liancheng:
@rdblue, shall we resolve this since you've fixed it in PR #367?

[asfimport]

Ryan Blue / @rdblue:
We shouldn't close this yet because we're working on adding sort order to the format. See the new link to Format PR #46.

[asfimport]

Julian Hyde:
It seems to me that the correct way to represent string orderings is to adhere to the same specification as the COLLATE in a standard SQL data type.

Quoting to a stackoverflow question about the COLLATE clause in Microsoft SQL Server, SQL_Latin1_General_CP1_CI_AS
breaks up into interesting parts:

• latin1 makes the server treat strings using charset latin 1, basically ascii
• CI case insensitive comparisons so 'ABC' would equal 'abc'
• AS accent sensitive, so 'ü' does not equal 'u'

I couldn't find an ISO definition of collation, but several databases support it, including MySQL and Oracle 12, so I suspect it is pretty well defined.

[asfimport]

Ryan Blue / @rdblue:
[~julianhyde], I'm not sure that we should make this SQL-specific. Right now, the PR allows a String identifier for custom sort orders, so this example could be expressed as "sql:SQL_Latin1_General_CP1_CI_AS". Does that work?

Also, I'd like to find a spec for these, even if it is just a page documenting what postgres or some other implementation does.

*Edit*: Fixed the example, since SQL_Latin1_General_CP1_CI_AS is one collating sequence

[asfimport]

Julian Hyde:
If you're following the relevant industry standard you're not being SQL specific. I don't know whether there is an ISO standard, but even if there isn't, we'd be well advised to follow what the big DBs do. The folks at SQL Server and Oracle have had more time to identify the appropriate standard to follow. If we roll our own standard we are very likely to come up with something worse.

[asfimport]

Ryan Blue / @rdblue:
The standard I found is ICU 54: http://userguide.icu-project.org/collation/api#TOC-Instantiating-the-Predefined-Collators

I agree that we should support what SQL engines do, but I don't agree that they've necessarily chosen an appropriate or "correct" one. What they do is widely used enough to be a de-facto standard, but I don't think that should preclude supporting other standards.

[asfimport]

Julian Hyde:
In Calcite, a collation is a character set, a dollar separator, a locale, a dollar separator, and a strength. Thus: COLLATE "ISO-8859-1$en_US$primary". I have not had time to check how/whether that jibes with the SQL standard. It does seem to align somewhat with java.text.Collator, which has locale and strength.

[asfimport]

Julien Le Dem / @julienledem:
The first issue of not returning bad stats was solved in: apache/parquet-java@de99127

[asfimport]

Zoltan Ivanfi / @zivanfi:
PR #46 went through a lot of changes during the code review and ended up being entirely different than the original thinking. However, when it got committed, some parts of it still contained references to the original solution that got abandoned. I created PR #55 to clean up these inconsistencies.

Can I reopen this issue for this change or should I create a new one for these clarifications?

[asfimport]

Ryan Blue / @rdblue:
Merged PR #55 with some follow-ups. Thanks for the fix, @zivanfi!

[asfimport]

Ivan Gozali:
Hi all, just curious if this issue is also resolved in Parquet 1.8.2, which is what Spark 2.2.0 uses?