Skip to content
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

Fix format_datetime to used linked TimeZone names #11337

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/linux-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ jobs:
- name: Make Debug Build
env:
VELOX_DEPENDENCY_SOURCE: BUNDLED
ICU_SOURCE: SYSTEM
MAKEFLAGS: "NUM_THREADS=8 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=3"
EXTRA_CMAKE_FLAGS: "-DVELOX_ENABLE_ARROW=ON -DVELOX_ENABLE_PARQUET=ON"
run: |
Expand Down
29 changes: 6 additions & 23 deletions CMake/resolve_dependency_modules/boost.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,13 @@
# limitations under the License.
include_guard(GLOBAL)

if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
if(ON_APPLE_M1)
list(APPEND CMAKE_PREFIX_PATH "/opt/homebrew/opt/icu4c")
else()
list(APPEND CMAKE_PREFIX_PATH "/usr/local/opt/icu4c")
endif()
endif()

# ICU is only needed with Boost build from source
set_source(ICU)
resolve_dependency(
ICU
COMPONENTS
data
i18n
io
uc
tu
test)

add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/boost)
if(${ICU_SOURCE} STREQUAL "BUNDLED")
# ensure ICU is built before Boost
add_dependencies(boost_regex ICU ICU::i18n)

if(ICU_SOURCE)
if(${ICU_SOURCE} STREQUAL "BUNDLED")
# ensure ICU is built before Boost
add_dependencies(boost_regex ICU ICU::i18n)
endif()
endif()

# This prevents system boost from leaking in
Expand Down
19 changes: 19 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,25 @@ endif()

set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

if(CMAKE_SYSTEM_NAME MATCHES "Darwin")
if(ON_APPLE_M1)
list(APPEND CMAKE_PREFIX_PATH "/opt/homebrew/opt/icu4c")
else()
list(APPEND CMAKE_PREFIX_PATH "/usr/local/opt/icu4c")
endif()
endif()

set_source(ICU)
resolve_dependency(
ICU
COMPONENTS
data
i18n
io
uc
tu
test)

set(BOOST_INCLUDE_LIBRARIES
atomic
context
Expand Down
1 change: 0 additions & 1 deletion scripts/setup-ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,3 @@ function install_apt_deps {
fi
fi
)

30 changes: 30 additions & 0 deletions velox/external/date/patches/0006-add_get_time_zone_names.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
diff --git a/velox/external/date/tz.cpp b/velox/external/date/tz.cpp
--- a/velox/external/date/tz.cpp
+++ b/velox/external/date/tz.cpp
@@ -3538,6 +3538,14 @@
return get_tzdb_list().front();
}

+std::vector<std::string> get_time_zone_names() {
+ std::vector<std::string> result;
+ for (const auto& z : get_tzdb().zones) {
+ result.push_back(z.name());
+ }
+ return result;
+}
+
const time_zone*
#if HAS_STRING_VIEW
tzdb::locate_zone(std::string_view tz_name) const
diff --git a/velox/external/date/tz.h b/velox/external/date/tz.h
--- a/velox/external/date/tz.h
+++ b/velox/external/date/tz.h
@@ -1258,6 +1258,8 @@

DATE_API const tzdb& get_tzdb();

+std::vector<std::string> get_time_zone_names();
+
class tzdb_list
{
std::atomic<tzdb*> head_{nullptr};
8 changes: 8 additions & 0 deletions velox/external/date/tz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3538,6 +3538,14 @@ get_tzdb()
return get_tzdb_list().front();
}

std::vector<std::string> get_time_zone_names() {
std::vector<std::string> result;
for (const auto& z : get_tzdb().zones) {
result.push_back(z.name());
}
return result;
}

const time_zone*
#if HAS_STRING_VIEW
tzdb::locate_zone(std::string_view tz_name) const
Expand Down
2 changes: 2 additions & 0 deletions velox/external/date/tz.h
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,8 @@ operator<<(std::ostream& os, const tzdb& db);

DATE_API const tzdb& get_tzdb();

std::vector<std::string> get_time_zone_names();

class tzdb_list
{
std::atomic<tzdb*> head_{nullptr};
Expand Down
3 changes: 2 additions & 1 deletion velox/functions/lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ velox_link_libraries(velox_functions_util velox_vector velox_common_base)
velox_add_library(velox_functions_lib_date_time_formatter DateTimeFormatter.cpp
DateTimeFormatterBuilder.cpp)

velox_link_libraries(velox_functions_lib_date_time_formatter velox_type_tz)
velox_link_libraries(velox_functions_lib_date_time_formatter velox_type_tz
ICU::i18n ICU::uc)

velox_add_library(
velox_functions_lib
Expand Down
Loading
Loading