feat: implement Literal Transform #156
Conversation
| @@ -51,6 +61,45 @@ Result<ArrowArray> BucketTransform::Transform(const ArrowArray& input) { | |||
| return NotImplemented("BucketTransform::Transform"); | |||
| } | |||
|
|
|||
| Result<std::optional<Literal>> BucketTransform::Transform(const Literal& literal) { | |||
| assert(literal.type() == source_type()); | |||
There was a problem hiding this comment.
Not related to this PR: we should introduce something like ARROW_DCHECK to add some explicit debug-level checks.
There was a problem hiding this comment.
Agreed, we may incorporate this alongside our logging infrastructure.
also fix some lint warnings
zhjwpku
force-pushed
the
implement_transform
branch
from
853fab3 to
4c0766a
Compare
| using namespace std::chrono; // NOLINT | ||
| switch (source_type()->type_id()) { | ||
| case TypeId::kDate: { | ||
| auto value = std::get<int32_t>(literal.value()); |
There was a problem hiding this comment.
nit: I think we need also move all of these transform functions to a dedicated utility file and add exhaustive test cases. Could you create an issue for this?
|
@Fokko @zeroshade Could you help review this? Thanks! |
That would require introducing a temporary variable before the return statement. I think if we have thorough test cases, the assert might not be necessary. |
src/iceberg/util/truncate_utils.h
Outdated
|
|
||
| namespace iceberg { | ||
|
|
||
| ICEBERG_EXPORT class TruncateUtils { |
There was a problem hiding this comment.
| ICEBERG_EXPORT class TruncateUtils { | |
| class ICEBERG_EXPORT TruncateUtils { |
There was a problem hiding this comment.
I initially used this approach, but the pre-commit clang-format hook misformatted the file, so I followed the StringUtils approach instead.
There was a problem hiding this comment.
Ok, I found that upgrade https://github.com/pre-commit/mirrors-clang-format to v20.1.8 works, so changed to the suggested approach, thanks.
No description provided.