Skip to content

Conversation

@youssef-tharwat
Copy link
Contributor

Summary

  • Add support for toLower/toUpper (Cypher) and lower/upper (SQL) string functions
  • Change fallback for unsupported functions from lit(0) to ScalarValue::Null to prevent type coercion errors

Problem

When using toLower(s.name) CONTAINS 'x' with integer columns in the RETURN clause, a type coercion error occurred:

type_coercion: There isn't a common type to coerce Int32 and Utf8 in LIKE expression

This happened because toLower was not implemented and fell through to the default case which returned lit(0) (Int32). When combined with CONTAINS (translated to LIKE), DataFusion couldn't coerce Int32 to Utf8.

Solution

  1. Implemented toLower/toUpper using DataFusion's lower()/upper() functions
  2. Changed the fallback for unsupported functions from lit(0) to ScalarValue::Null, which coerces to any type in SQL semantics

Test plan

  • Added unit tests for toLower, toUpper, lower, upper functions
  • Added integration test reproducing the exact bug scenario
  • All 196 existing tests pass

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2026

Codecov Report

❌ Patch coverage is 89.24731% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...t/lance-graph/src/datafusion_planner/expression.rs 89.24% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Collaborator

@ChunxuTang ChunxuTang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@youssef-tharwat Thanks so much for the fix!
I just left very minor comments for the tests. The fix generally looks good to me.

Meanwhile, could you fix the style check error of your PR? Feel free to tag me when you want me to review again.

@youssef-tharwat
Copy link
Contributor Author

@ChunxuTang all done :)

Copy link
Collaborator

@ChunxuTang ChunxuTang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the contribution.

@ChunxuTang
Copy link
Collaborator

@youssef-tharwat One minor thing: there's a style check error reported by the GitHub workflows. Could you run cargo fmt --manifest-path Cargo.toml locally to fix the error and update your PR?

youssef-tharwat and others added 4 commits January 13, 2026 16:11
- Add support for toLower/toUpper (Cypher) and lower/upper (SQL) string functions
- Change fallback for unsupported functions from lit(0) to ScalarValue::Null
  to prevent type coercion errors in any context (string or numeric)
- Add unit tests for new string functions
- Add integration tests including exact bug reproduction

Fixes type coercion error when using toLower(s.name) CONTAINS 'x' with
integer columns in RETURN clause.
Co-Authored-By: Claude <noreply@anthropic.com>
@youssef-tharwat youssef-tharwat force-pushed the fix/string-functions-support branch from 6984153 to 2ff66f0 Compare January 13, 2026 15:16
@ChunxuTang ChunxuTang merged commit 357c8aa into lance-format:main Jan 13, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants