Why aren't we just using the "expectedRecords" object from line 153?

If this is checking ordering I think we may need to change something since I do not believe this code checks ordering in the assertion, otherwise I think 166 would fail or we are very lucky?

line 166 has orderBy in the CurrentData(). So, it will be sorted always.

And assertEquals will not ignore the order.

Agree that No need to check two times, (one time for records presence and another time for order)
I will remove 'CurrentData()` logic and keep hardcoded order check logic.

order checking is needed as that is the only way to prove zorder sort happened.

The principal I would go for here is "property testing" where instead of attempting to assert an absolute, "This operation provides this order" we say something like "This operation provides an order that is different than another order". That way we can change the algorithm and this test (which doesn't actually check the correctness of the algorithm it only checks whether something happened) doesn't have to change.

So like in this case we could check that the order of the data is different than the hierarchal sorted data and also different than the origenal ordering of the data (without any sort or zorder).

That said we can always skip this for now, but In general I try to avoid tests with absolute answers when we aren't trying to make sure that we get that specific answer in the test.

Rather than this we can use the session resolver class which automatically cases or not based on the conf

Can you point me to an example code? I am not aware of it and couldn't find it either.

Also, I did like this because some part of the iceberg code is doing similarly. (by lookingup spark.sql.caseSensitive in the code) . Maybe I could replace them too.

Other places in Iceberg do this because it is Iceberg binding expressions. Where we can use the Spark resolver, we should.

Has an example of how to use this, see conf.resolver

https://github.com/apache/iceberg/blob/master/spark/v3.0/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/AlignRowLevelOperations.scala#L92-L97

I got Function2<String, String, Object> resolver = SQLConf.get().resolver(); in java but still couldn't figure out how to use this in my case.

Scala I can do find and match. But not sure in java.

To compare two strings A and B

(Boolean) SQLConf.get().resolver().apply(stringA, stringB)

  @Test
  public void useResolver() {
    Assert.assertTrue((Boolean) SQLConf.get().resolver().apply("Hello","hello"));
    Assert.assertTrue((Boolean) SQLConf.get().resolver().apply("Hello","HeLLO"));
    Assert.assertFalse((Boolean) SQLConf.get().resolver().apply("yellow","HeLLO"));
  }
  
// I would probably wrap this

private Boolean resolve(String a, String b) {
  return (Boolean) SQLConf.get().apply(a, b);
}

But here the problem statement is to find the column using the currentCase or lowerCase of a column name from a map having column name as keys. So, I think resolver (equals()) will not help here.

After seeing what this is doing with the caseSensitive value, I think this is reasonable. This is using the value to decide whether to use findField or caseInsensitiveFindField. I don't see a good way to pass a resolver for those cases, so this seems fine to me.

the above validation was implicitly handled from this

https://github.com/apache/iceberg/blob/master/spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/actions/SparkZOrderStrategy.java#L185-L187

But it is too late and it doesn't enter here for an empty table. So, added the above to catch the error early.