Skip to content

feat: add toLower/toUpper support, fix type coercion error#82

Merged
ChunxuTang merged 4 commits into
lance-format:mainfrom
noodlbox:fix/string-functions-support
Jan 13, 2026
Merged

feat: add toLower/toUpper support, fix type coercion error#82
ChunxuTang merged 4 commits into
lance-format:mainfrom
noodlbox:fix/string-functions-support

Conversation

@youssef-tharwat

Copy link
Copy Markdown
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

codecov-commenter commented Jan 10, 2026

Copy link
Copy Markdown

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!

@ChunxuTang ChunxuTang left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment thread rust/lance-graph/src/datafusion_planner/expression.rs Outdated
Comment thread rust/lance-graph/tests/test_datafusion_pipeline.rs Outdated
Comment thread rust/lance-graph/tests/test_datafusion_pipeline.rs Outdated
@youssef-tharwat

Copy link
Copy Markdown
Contributor Author

@ChunxuTang all done :)

@ChunxuTang ChunxuTang left a comment

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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