test: enable native path in lower/upper_enabled sql fixtures

[cestercian]

Which issue does this PR close?

Fixes apache#5611

Rationale for this change

lower_enabled.sql and upper_enabled.sql were meant to cover the native case-conversion path, but they set spark.comet.expression.Lower/Upper.allowIncompatible=true. That config is a no-op: CometCaseConversionBase never reports Incompatible, so allowIncompatible is never consulted. The real switch is spark.comet.caseConversion.enabled.

The fixtures therefore exercised the same codegen dispatcher path as lower.sql / upper.sql. ASCII inputs produce identical results on both paths, so the drift was not caught.

What changes are included in this PR?

In both spark/src/test/resources/sql-tests/expressions/string/lower_enabled.sql and upper_enabled.sql:

• Change the Config directive to spark.comet.caseConversion.enabled=true
• Update the header comments so they describe the native case-conversion path rather than the allowIncompatible opt-in

ASCII inputs are unchanged. expect_native(...) annotations are left for after apache#5610 lands.

How are these changes tested?

Ran against Spark 4.1 (project default) after make core:

./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite lower_enabled" -Dtest=none
./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite upper_enabled" -Dtest=none

Both passed (sql-file: expressions/string/lower_enabled.sql and sql-file: expressions/string/upper_enabled.sql; 1 test each, 0 failures).

The Config key matches CometConf.COMET_CASE_CONVERSION_ENABLED. ASCII lower/upper results are identical on the native and dispatcher paths, so existing query comparisons remain valid.

expect_native assertions are intentionally not added here; they depend on apache#5610.

  

Summary by CodeRabbit

• Documentation
  • Updated test descriptions for lower() and upper() to accurately document the native case-conversion path.
  • Clarified that the documented behavior applies to ASCII-only inputs and is enabled through the case-conversion setting.
  • No functional or query behavior changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 3c47ddf2-7dde-4e57-849c-7ad27d8d17ad

📥 Commits

Reviewing files that changed from the base of the PR and between 1e10eed and 79decad.

📒 Files selected for processing (2)

• spark/src/test/resources/sql-tests/expressions/string/lower_enabled.sql
• spark/src/test/resources/sql-tests/expressions/string/upper_enabled.sql

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The pull request updates header comments in the lower() and upper() SQL test resources. The comments now describe the native ASCII case-conversion paths and use spark.comet.caseConversion.enabled=true. Test queries remain unchanged.

Changes

Case conversion test descriptions

Layer / File(s)	Summary
Update case conversion test comments spark/src/test/resources/sql-tests/expressions/string/lower_enabled.sql, spark/src/test/resources/sql-tests/expressions/string/upper_enabled.sql	The comments now describe the native ASCII case-conversion paths and the spark.comet.caseConversion.enabled=true configuration. No test queries changed.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 79dec

This localized change enables the intended native case-conversion path in two test fixtures without changing production behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: andygrove

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: enabling the native case-conversion path in the lower and upper SQL fixtures.
Description check	✅ Passed	The description includes the issue reference, rationale, changed files and configuration, testing commands and results, and the reason native expectation annotations are deferred.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/enable-native-lower-upper-541e

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}