Fokko commented on code in PR #8267:
URL: https://github.com/apache/iceberg/pull/8267#discussion_r1289601328
##########
python/pyiceberg/expressions/literals.py:
##########
@@ -139,7 +139,7 @@ def literal(value: L) -> Literal[L]:
elif isinstance(value, str):
return StringLiteral(value)
elif isinstance(value, UUID):
- return UUIDLiteral(value)
+ return UUIDLiteral(value) # type: ignore
Review Comment:
So, the `UUIDLiteral` is not intended to be used by the user. The user would
pass in:
```python
>>> l = literal('27f6397c-65b9-4464-a2f5-f4ee81629134')
StringLiteral('27f6397c-65b9-4464-a2f5-f4ee81629134')
```
And then internally, when binding to the schema, it will be converted into
an UUID, which is failing right now:
```python
>>> from pyiceberg.types import UUIDType
>>> l.to(UUIDType)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File
"/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/functools.py",
line 914, in _method
return method.__get__(obj, cls)(*args, **kwargs)
File
"/Users/fokkodriesprong/Desktop/iceberg/python/pyiceberg/expressions/literals.py",
line 519, in to
raise TypeError(f"Cannot convert StringLiteral into {type_var}")
TypeError: Cannot convert StringLiteral into <class
'pyiceberg.types.UUIDType'>
```
Which means that we need to implement this one as well.
The same is for the bytes:
```python
>>> from uuid import UUID
>>> u = UUID('27f6397c-65b9-4464-a2f5-f4ee81629134')
>>> l = literal(u.bytes)
>>> l
BinaryLiteral(b"'\xf69|e\xb9Dd\xa2\xf5\xf4\xee\x81b\x914")
>>> l.to(UUIDType)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File
"/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/functools.py",
line 914, in _method
return method.__get__(obj, cls)(*args, **kwargs)
File
"/Users/fokkodriesprong/Desktop/iceberg/python/pyiceberg/expressions/literals.py",
line 640, in to
raise TypeError(f"Cannot convert BinaryLiteral into {type_var}")
TypeError: Cannot convert BinaryLiteral into <class
'pyiceberg.types.UUIDType'>
```
I think we need to extend the literal constructor with many more types, such
as `datetime`, `date`, `time`, `UUID`, etc. For the datetime related, I've
created a separate issue: https://github.com/apache/iceberg/issues/8282 Maybe
we can add `UUID` to `literal(..)` in this PR to avoid backward incompatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
