Spark-3.5: make where sql case sensitive setting alterable in rewrite data files procedure

[ludlows]

this pr aims to make the rewriteDataFile action is aware of the user settings about sql case sensitivity in the where statement.
the implementation is simple.
we first obtain the case sensitive setting and save it as a variable in the constructor of rewriteDataFileAction.
then, we pass the variable to the tableScan .

related issue: #11438

[singhpk234]

@ludlows can you please also add an UT for it to future proof it ?

[szehon-ho]

Yes looks reasonable to me as well, agree with @singhpk234 about UT

[ludlows]

Hi @szehon-ho ,
please review the test cases, should you have time.
one possible problem is the type of exception is IllegalArgumentException here instead of the ValidationException mentioned in the issue.

[ludlows]

I think I typed the wrong version of iceberg in the issue #11438

[huaxingao]

@ludlows
Thanks for the PR. I have a couple of questions:

1. It seems to me that the tests don't really test the changes in this PR; they would pass even without the fix. I think we should add some tests that would fail without the fix but can pass with it.
2. Do we want to make the Spark SQL configuration spark.sql.caseSensitive apply to Iceberg stored procedure parameters? If so, we probably should apply spark.sql.caseSensitive to all Iceberg stored procedure parameters. Are there other Iceberg stored procedure parameters that should also honor spark.sql.caseSensitive?

[ludlows]

1. It seems to me that the tests don't really test the changes in this PR; they would pass even without the fix. I think we should add some tests that would fail without the fix but can pass with it.
2. Do we want to make the Spark SQL configuration spark.sql.caseSensitive apply to Iceberg stored procedure parameters? If so, we probably should apply spark.sql.caseSensitive to all Iceberg stored procedure parameters. Are there other Iceberg stored procedure parameters that should also honor spark.sql.caseSensitive?

---

hi @huaxingao ,
thanks for the questions above.
about the 2nd one, as a data engineer, our users were asking me : why not all parts in the procedure are case insensitive even I have set spark.sql.caseSensitive to false? since the procedure is triggered at the sql level but why the parameter of procedure is not affected?

I think it is reasonable to apply the setting about the sql case sensitivity to all procedures, but we could take this pr as the starting point.

[huaxingao]

@ludlows Thanks for the quick fix. Can we have a test that fails without the fix but passes with it? It seems that all your current tests pass even without the fix.

[ludlows]

@huaxingao I think the test method testFilterCaseSensitivityBeforeChange() (leading to validation exception) has shown the bug exists before the PR.

[huaxingao]

@ludlows I think you can simply reproduce the problem by something like

    createTable();
    insertData(10);
    sql("SET %s=false", SQLConf.CASE_SENSITIVE().key());
    sql("CALL %s.system.rewrite_data_files(table=>'%s', where=>'C1 > 0'), catalogName, tableIdent));

[ludlows]

@huaxingao thanks for the comment.
but i don't think the problem will be raised since the bug has been fixed by this PR.
please check the test code belove:

@TestTemplate
  public void testFilterCaseSensitivityAfterChange() {
    createTable();
    insertData(10);
    sql("set spark.sql.caseSensitive=false");
    assertEquals(
        "Should have done nothing but passed the schema validation, since no files are present",
        ImmutableList.of(row(0, 0, 0L, 0)),
        sql(
            "CALL %s.system.rewrite_data_files(table=>'%s', where=>'C1 > 90000000')",
            catalogName, tableIdent));
  }

the test case above has passed .

[huaxingao]

@ludlows Thanks for the quick reply. I know my example will pass with the PR's fix. However, the problem will arise without the fix. We need a simple test that fails without the fix and passes with it. A straightforward test like my example should suffice, with minimal changes. My goal is to keep the test as simple as possible.

[huaxingao]

LGTM

[szehon-ho]

Thanks @ludlows , and also @huaxingao, @anuragmantri @singhpk234 for reviews