I think there are still a few issues in this doc

1. This should only produce a single manifest list.
2. Unclear what this is supposed to produce, are we sorting or clustering here? Will manifests be separated based on values of B and D? Does the order matter?

Rewriting Manifests in this way
   * will yield manifest_lists that point to manifest_files containing data files for common 'd' and
   * 'b' partitions.

I think the core message here is
Rewrites the manifests and order (cluster?) their entries based on the specified transforms. Manifests can be filtered based on their min and max partition values. Choosing a frequently queried partition field can reduce planning time by skipping unnecessary manifests.

Or something along those lines

This also shouldn't use the current spec but the one specified by specId(int specId)

Can update documentation. Thanks!

why cluster and not sort?

In the original proposal we talked about updating the Spark api to match the Java API, where they have a clusterBy interface

Educate me - I see clustering and sorting as synonyms. Clustering is just the word for the spark technique to sort and repartitionByRange data into "clusters". I'm not married to either.

Hmm that's a good question. In my eyes we are doing a hierarchical sort which feels different to me than a multi-dimensional clustering algo. So for example Cluster(a, b) might get me manifests with common tuples where A and B are correlated but we can't actually do that here.

So for example if I would expect cluster to make files like

{(1,1)(1,2)(2,1)(2,2)} 
{(1,3)(1,4)(2,3)(2,4)}
{(3,1)(3,2)(4,1)(4,2)}
{(3,3)(3,4)(4,3)(4,4)}

I would consider that clustered

But our current algo can't do that, it can only do a hierarchical sort, each column is dependent on the one before it. Like in the above example if I cluster (a,b) I would produce

{(1,1)(1,2)(1,3)(1,4)}
{(2,1)(2,2)(2,3)(2,4)}
{(3,1)(3,2)(3,3)(3,4)}
{(4,1)(4,2)(4,3)(4,4)}

Good illustration. I think the question in your illustration is - are the tuples globally sorted? And does that matter? I don't think the manifest list assumes sorting, so each manifest's lower/uppers are checked. In which case I'd call it "clustering".

Really no preference on naming. Like I said I just picked this to match the Java API. At the end of the day either of your examples is better than

{(1,4)(1,4)(1,4)(1,4)}
{(1,4)(1,4)(1,4)(1,4)}
{(1,4)(1,4)(1,4)(1,4)}
{(1,4)(1,4)(1,4)(1,4)}

:-)

The original API "clusters" this api "sorts", I'm not sure they are comparable. I really think the naming should be "sort"

Can do

This shouldn't be the current spec, but the spec chosen for rewrite target

The checks here are pretty complicated, I think it probably would be a lot simpler to just check that the rows within the manifest are ordered rather than going through all the manifest metadata. Our readers should already read through entries in order from the file I think ...

Then we could just check "contents before = contents after" - "row before unsorted on c3 != rows after sorted on c3 transform"

I'll see what I can do. Contents can still be sorted locally, but not globally sorted without the clusterBy. I think you're suggesting to just say "sorted but different" is sufficient?

I'm just trying to think how we can avoid re-implementing a check overlapping bounds function (we already have done this once in the spark rewrite datafiles test code - but for metrics) and avoid doing all the conversions from byte buffer. It's a lot of code to have in a single test case and I think it's a bit difficult to check.

Since you are using a Integer output here we probably could just use the MetadataTable - ManfiestsTable which reads the manifest list and serailzes all the partition min/maxs into strings. So you could take those flatten them into a list and check that the list is sorted

for (lower, upper <- partitionsummaries.field you want) {
   bounds.add(Integer.of(lower))
   bounds.add(Integer.of(upper))
}

assert that bounds is sorted

Mostly I'm looking here for a way to simplify this test so it's more clear what's doing without so much boiler plate code. If you have other suggestions I'm open those as well

I'd also like to apologize for taking so long to get back to this, please feel free to ping my on slack if I haven't noticed a github comment. My email is a bit packed so sometimes I lose important notices.

Will simplify, thanks for staying plugged in!

Here and below we should be using "sort" and not "cluster"

Updated documentation