-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for timestamps #70
Changes from 6 commits
f85ba70
423082e
425d68b
1e37716
5715cfd
05adbcc
62a5da0
3154eac
b906a45
7accf0a
2f961ef
0721e0c
e13ae41
411eb4c
5e883bb
a85ecaa
a554e0f
6a37966
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
// Copyright 2024 Man Group Operations Limited | ||
Check notice on line 1 in include/sparrow/data_type.hpp GitHub Actions / buildRun clang-format on include/sparrow/data_type.hpp
|
||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
|
@@ -14,6 +14,16 @@ | |
|
||
#pragma once | ||
|
||
#include <version> | ||
#include <chrono> | ||
|
||
#if __cpp_lib_chrono > 201907L | ||
// P0355R7 (Extending chrono to Calendars and Time Zones) has not been entirely implemented in libc++ yet. | ||
// See: https://libcxx.llvm.org/Status/Cxx20.html#note-p0355 | ||
// For now, we use HowardHinnant/date as a replacement if we are compiling with libc++. | ||
// TODO: remove this once libc++ has full support for P0355R7. | ||
# include <date/tz.h> | ||
#endif | ||
#include <climits> | ||
#include <cstdint> | ||
#include <optional> | ||
|
@@ -45,6 +55,12 @@ | |
using float64_t = std::float64_t; | ||
#endif | ||
|
||
#if __cpp_lib_chrono <= 201907L | ||
using timestamp = std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds>; | ||
#else | ||
using timestamp = date::zoned_time<std::chrono::system_clock, std::chrono::nanoseconds>; | ||
#endif | ||
|
||
jjerphan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// We need to be sure the current target platform is setup to support correctly these types. | ||
static_assert(sizeof(float16_t) == 2); | ||
static_assert(sizeof(float32_t) == 4); | ||
|
@@ -82,14 +98,15 @@ | |
BINARY, | ||
// Fixed-size binary. Each value occupies the same number of bytes | ||
FIXED_SIZE_BINARY, | ||
// Number of nanoseconds since the UNIX epoch with an optional timezone. | ||
// See: https://arrow.apache.org/docs/python/timestamps.html#timestamps | ||
TIMESTAMP, | ||
}; | ||
|
||
/// C++ types value representation types matching Arrow types. | ||
// NOTE: this needs to be in sync-order with `data_type` | ||
using all_base_types_t = mpl::typelist< | ||
std::nullopt_t // REVIEW: not sure about if we need to have this one? for representing NA? is this | ||
// the right type? | ||
, | ||
std::nullopt_t, | ||
bool, | ||
std::uint8_t, | ||
std::int8_t, | ||
|
@@ -103,7 +120,8 @@ | |
float32_t, | ||
float64_t, | ||
std::string, | ||
std::vector<byte_t> | ||
std::vector<byte_t>, | ||
sparrow::timestamp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there is one type missing before that one, see compare with the order of the enum, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took the values from Arrow implementation, not sure which type this value should match. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, must we work on supporting Edit: I guess we could support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The support for DATE32 and DATE64 can be added later. Explicit integer identifiers would be required later for backward compatibility, but it doe snot matter for now, nor the enum "stability". |
||
// TODO: add missing fundamental types here | ||
>; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__cpp_lib_chrono
(like any other FTM) might not be implemented in compiler, and it thus unusable.See the discussions in llvm/llvm-project#73953.