Fix UUID empty string handling in UPDATE operations #3106
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where UPDATE operations on tables with UUID columns containing NULL values would fail with "invalid value for type uuid" errors. The fix ensures that empty strings from the underlying storage are correctly interpreted as NULL values for UUID columns during UPDATE operations.
Key Changes:
- Added empty string handling for UUID columns in table scan and array-to-datum conversion functions
- Added comprehensive test coverage for UUID NULL handling in UPDATE operations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| postgres/tests/py_tests/test_uuid_null_update_bug.py | New test file validating UUID NULL handling in UPDATE operations across various scenarios |
| cpp/deeplake_pg/table_scan_impl.hpp | Added empty string check to treat empty UUIDs as NULL in table scans |
| cpp/deeplake_pg/nd_utils.hpp | Added empty string check to treat empty UUIDs as NULL in array-to-datum conversions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Treat empty string as NULL for UUID columns (same as duckdb_deeplake_scan.cpp) | ||
| if (str.empty()) { | ||
| // Return NULL datum - caller must set is_null flag appropriately | ||
| return (Datum)0; | ||
| } |
There was a problem hiding this comment.
The function returns a NULL datum but relies on the caller to set the is_null flag appropriately. This creates an ambiguous return value where (Datum)0 could represent either NULL or a valid zero datum. Consider returning a std::pair<Datum, bool> similar to table_scan::get_datum to explicitly communicate the null status, or document the expected caller behavior more clearly in the function signature.
🚀 🚀 Pull Request
Impact
Description
Things to be aware of
Things to worry about
Additional Context