fix: Avoid mistaken ILike to string equality optimization

[srh]

Which issue does this PR close?

• Closes ILike with no wildcards is mistakenly optimized to string equality #15835.

Rationale for this change

Bugfix

What changes are included in this PR?

Bugfix and unit test cases for the optimization.

Are these changes tested?

Yes.

Are there any user-facing changes?

This changes the behavior of ILIKE.

[comphead]

Thanks @srh for your first contribution. Wondering if the bug you fixing related to parser like #15820 or to the query correctness?

What I mean for this fix would be that possible to test the change as a SQL query?

[srh]

@comphead It isn't parser related, it's a bug in evaluation, with this optimizer.

It seems a query like "SELECT s.col ILIKE 'A' FROM (SELECT 'a' AS col) AS s" will trigger the bug. The query SELECT 'a' ILIKE 'A' evaluates to true because it hits constant folding before ExprSimplifier.

Here is a complete program that reproduces this on 46.0.1:

use datafusion::{
    arrow::
        util::pretty,
    prelude::*,
};
use tokio;

#[tokio::main]
async fn main() -> datafusion::error::Result<()> {
    let session_config = SessionConfig::new();
    let ctx = SessionContext::new_with_config(session_config);

    for value in &["a", "A"] {
        let query = format!("SELECT s.col ILIKE 'A' FROM (SELECT '{}' AS col) AS s", value);

        let df = ctx.sql(&query).await?;
        let results = df.collect().await?;

        println!("'{}' ILIKE 'A' result:", value);
        pretty::print_batches(&results)?;
    }
    Ok(())
}

Output on 46.0.1 (before applying this change)

'a' ILIKE 'A' result:
+-----------------------+
| s.col ILIKE Utf8("A") |
+-----------------------+
| false                 |
+-----------------------+
'A' ILIKE 'A' result:
+-----------------------+
| s.col ILIKE Utf8("A") |
+-----------------------+
| true                  |
+-----------------------+

[comphead]

Thanks @srh for providing the test case please add the query to one of select.slt files to preserve the regression

[alamb]

looks like we have completed the TODO as well -- can yoy remove this comment?

[srh]

I think that comment is referring to parsing escape characters and performing the optimization on strings with character escapes too.

[alamb]

Thank you for this PR @srh

[srh]

Thanks @srh for providing the test case please add the query to one of select.slt files to preserve the regression

I have added a test case (not the one with an inner select, because there is a table already set up to use with a non-constant expression) and have confirmed it fails when the fix is reverted.

[alamb]

Thank you @srh - this looks good to me

[alamb]

In case anyone is curious, without the fix this test fails like this:

Completed 1 test files in 0 seconds                                                                                                                                                                                             External error: query result mismatch:
[SQL] SELECT s FROM test WHERE s ILIKE 'p1';
[Diff] (-expected|+actual)
-   P1
    p1
at test_files/strings.slt:118

[comphead]

Thanks @srh for your contribution.