From 03d63e96242d7888d55d64f9735d3e60606fefd1 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 4 May 2025 07:25:48 +0900 Subject: [PATCH] GH-46299: [C++][Compute] Don't use `static inline const` for default options We use static variables for compute function option types. For example: https://github.com/apache/arrow/blob/068416bd411d6a8e2949f8ebcb2f80e2c302ef6b/cpp/src/arrow/compute/api_vector.cc#L127-L170 These option types are used for compute function options. For example: https://github.com/apache/arrow/blob/068416bd411d6a8e2949f8ebcb2f80e2c302ef6b/cpp/src/arrow/compute/api_vector.cc#L237 If we use `static inline const` for compute function options, these compute function options may be initialized BEFORE compute function option types. We use `static inline const` only for `rank`, `rank_quantile` and `rank_normal`: https://github.com/apache/arrow/blob/068416bd411d6a8e2949f8ebcb2f80e2c302ef6b/cpp/src/arrow/compute/kernels/vector_rank.cc#L382-L386 https://github.com/apache/arrow/blob/068416bd411d6a8e2949f8ebcb2f80e2c302ef6b/cpp/src/arrow/compute/kernels/vector_rank.cc#L399-L403 https://github.com/apache/arrow/blob/068416bd411d6a8e2949f8ebcb2f80e2c302ef6b/cpp/src/arrow/compute/kernels/vector_rank.cc#L416-L420 We can avoid this initialization order problem by deferring default compute function options initialization. --- cpp/src/arrow/compute/kernels/vector_rank.cc | 24 ++++++++++++++----- .../arrow/compute/kernels/vector_sort_test.cc | 19 +++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/vector_rank.cc b/cpp/src/arrow/compute/kernels/vector_rank.cc index 81b7640b0fe..1338ebedbe9 100644 --- a/cpp/src/arrow/compute/kernels/vector_rank.cc +++ b/cpp/src/arrow/compute/kernels/vector_rank.cc @@ -381,9 +381,13 @@ class RankMetaFunction : public RankMetaFunctionBase { } RankMetaFunction() - : RankMetaFunctionBase("rank", Arity::Unary(), rank_doc, &kDefaultOptions) {} + : RankMetaFunctionBase("rank", Arity::Unary(), rank_doc, GetDefaultOptions()) {} - static inline const auto kDefaultOptions = RankOptions::Defaults(); + private: + static const RankOptions* GetDefaultOptions() { + static const auto kDefaultOptions = RankOptions::Defaults(); + return &kDefaultOptions; + } }; class RankQuantileMetaFunction : public RankMetaFunctionBase { @@ -398,9 +402,13 @@ class RankQuantileMetaFunction : public RankMetaFunctionBase { @@ -415,9 +423,13 @@ class RankNormalMetaFunction : public RankMetaFunctionBaseGetFunction("rank")); + ASSERT_STREQ(function->default_options()->type_name(), "RankOptions"); +} + TEST_F(TestRank, Real) { for (auto real_type : ::arrow::FloatingPointTypes()) { SetInput(ArrayFromJSON(real_type, "[2.1, 3.2, 1.0, 0.0, 5.5]")); @@ -2635,6 +2641,12 @@ class TestRankQuantile : public BaseTestRank { } }; +TEST_F(TestRankQuantile, DefaultOptions) { + ASSERT_OK_AND_ASSIGN(auto function, + GetFunctionRegistry()->GetFunction("rank_quantile")); + ASSERT_STREQ(function->default_options()->type_name(), "RankQuantileOptions"); +} + TEST_F(TestRankQuantile, Real) { for (auto type : ::arrow::FloatingPointTypes()) { AssertRankQuantileNumeric(type); @@ -2675,5 +2687,12 @@ TEST_F(TestRankQuantile, FixedSizeBinary) { AssertRankQuantile_N1N2N(); } +class TestRankNormal : public BaseTestRank {}; + +TEST_F(TestRankNormal, DefaultOptions) { + ASSERT_OK_AND_ASSIGN(auto function, GetFunctionRegistry()->GetFunction("rank_normal")); + ASSERT_STREQ(function->default_options()->type_name(), "RankQuantileOptions"); +} + } // namespace compute } // namespace arrow