From 2f4af55c8a69b65e7a2154391d44747222c7cb6b Mon Sep 17 00:00:00 2001 From: Ruth Kurniawati Date: Sun, 26 Oct 2025 19:21:17 -0400 Subject: [PATCH 1/7] Fix failure of FunctionMetadataTest.mod on MacOS Signed-off-by: Ruth Kurniawati --- .../functions/tests/FunctionMetadataTest.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp index 226133c9e437c..3892fc63d1889 100644 --- a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp +++ b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp @@ -40,6 +40,16 @@ class FunctionMetadataTest : public ::testing::Test { functionMetadata_ = getFunctionsMetadata(); } + static void sortConstraintArrays(json& metadata) { + for (auto const& [key, val] : metadata.items()) { + if (key.ends_with("Constraints") && metadata[key].is_array()) { + std::sort(metadata[key].begin(), metadata[key].end(), [](const json& a, const json& b) { + return a.dump() < b.dump(); + }); + } + } + } + void testFunction( const std::string& name, const std::string& expectedFile, @@ -59,6 +69,12 @@ class FunctionMetadataTest : public ::testing::Test { std::sort(expectedList.begin(), expectedList.end(), comparator); std::sort(metadataList.begin(), metadataList.end(), comparator); for (auto i = 0; i < expectedSize; i++) { + // Constraint arrays are coming from unordered map, they need to be sorted so that + // differences in the element order will not cause test failure. + + sortConstraintArrays(expectedList[i]); + sortConstraintArrays(metadataList[i]); + EXPECT_EQ(expectedList[i], metadataList[i]) << "Position: " << i; } } @@ -66,6 +82,7 @@ class FunctionMetadataTest : public ::testing::Test { json functionMetadata_; }; + TEST_F(FunctionMetadataTest, approxMostFrequent) { testFunction("approx_most_frequent", "ApproxMostFrequent.json", 7); } From 6ee41d76224edd41a2b58ee3cf9ed6bdf20a554d Mon Sep 17 00:00:00 2001 From: Ruth Kurniawati Date: Sun, 26 Oct 2025 20:10:01 -0400 Subject: [PATCH 2/7] Fix code formatting Signed-off-by: Ruth Kurniawati --- .../main/functions/tests/FunctionMetadataTest.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp index 3892fc63d1889..3517a473e3316 100644 --- a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp +++ b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp @@ -43,9 +43,10 @@ class FunctionMetadataTest : public ::testing::Test { static void sortConstraintArrays(json& metadata) { for (auto const& [key, val] : metadata.items()) { if (key.ends_with("Constraints") && metadata[key].is_array()) { - std::sort(metadata[key].begin(), metadata[key].end(), [](const json& a, const json& b) { - return a.dump() < b.dump(); - }); + std::sort( + metadata[key].begin(), + metadata[key].end(), + [](const json& a, const json& b) { return a.dump() < b.dump(); }); } } } @@ -69,8 +70,8 @@ class FunctionMetadataTest : public ::testing::Test { std::sort(expectedList.begin(), expectedList.end(), comparator); std::sort(metadataList.begin(), metadataList.end(), comparator); for (auto i = 0; i < expectedSize; i++) { - // Constraint arrays are coming from unordered map, they need to be sorted so that - // differences in the element order will not cause test failure. + // Constraint arrays are coming from unordered map, they need to be sorted + // so that differences in the element order will not cause test failure. sortConstraintArrays(expectedList[i]); sortConstraintArrays(metadataList[i]); @@ -82,7 +83,6 @@ class FunctionMetadataTest : public ::testing::Test { json functionMetadata_; }; - TEST_F(FunctionMetadataTest, approxMostFrequent) { testFunction("approx_most_frequent", "ApproxMostFrequent.json", 7); } From ad458248703a37eed08f14900f310d7ecd1abd5b Mon Sep 17 00:00:00 2001 From: Ruth Kurniawati Date: Mon, 27 Oct 2025 19:45:28 -0400 Subject: [PATCH 3/7] Consolidate sorting of constraint arrays in metadata and metadata elements into one helper function Signed-off-by: Ruth Kurniawati --- .../functions/tests/FunctionMetadataTest.cpp | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp index 3517a473e3316..73205314cad97 100644 --- a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp +++ b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp @@ -40,15 +40,20 @@ class FunctionMetadataTest : public ::testing::Test { functionMetadata_ = getFunctionsMetadata(); } - static void sortConstraintArrays(json& metadata) { - for (auto const& [key, val] : metadata.items()) { - if (key.ends_with("Constraints") && metadata[key].is_array()) { - std::sort( - metadata[key].begin(), - metadata[key].end(), - [](const json& a, const json& b) { return a.dump() < b.dump(); }); + static void sortMetadataList(json& list) { + for (auto& metadata : list) { + for (auto const& [key, val] : metadata.items()) { + if (key.ends_with("Constraints") && metadata[key].is_array()) { + std::sort( + metadata[key].begin(), + metadata[key].end(), + [](const json& a, const json& b) { return a.dump() < b.dump(); }); + } } } + std::sort(list.begin(), list.end(), [](const json& a, const json& b) { + return (a["outputType"] < b["outputType"]); + }); } void testFunction( @@ -62,20 +67,11 @@ class FunctionMetadataTest : public ::testing::Test { "/github/presto-trunk/presto-native-execution/presto_cpp/main/functions/tests/data/", expectedFile)); auto expected = json::parse(expectedStr); - auto comparator = [](const json& a, const json& b) { - return (a["outputType"] < b["outputType"]); - }; - json::array_t expectedList = expected[name]; - std::sort(expectedList.begin(), expectedList.end(), comparator); - std::sort(metadataList.begin(), metadataList.end(), comparator); + json expectedList = expected[name]; + sortMetadataList(expectedList); + sortMetadataList(metadataList); for (auto i = 0; i < expectedSize; i++) { - // Constraint arrays are coming from unordered map, they need to be sorted - // so that differences in the element order will not cause test failure. - - sortConstraintArrays(expectedList[i]); - sortConstraintArrays(metadataList[i]); - EXPECT_EQ(expectedList[i], metadataList[i]) << "Position: " << i; } } From f6c7646665f579204343d7f481481ee5a041b67b Mon Sep 17 00:00:00 2001 From: Ruth Kurniawati Date: Tue, 28 Oct 2025 09:56:52 -0400 Subject: [PATCH 4/7] Use folly::hasher in the sortMetadataList, change sortMetadataList parameter to json::array_t Signed-off-by: Ruth Kurniawati --- .../main/functions/tests/FunctionMetadataTest.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp index 73205314cad97..d481f7bd26a40 100644 --- a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp +++ b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp @@ -40,7 +40,7 @@ class FunctionMetadataTest : public ::testing::Test { functionMetadata_ = getFunctionsMetadata(); } - static void sortMetadataList(json& list) { + static void sortMetadataList(json::array_t& list) { for (auto& metadata : list) { for (auto const& [key, val] : metadata.items()) { if (key.ends_with("Constraints") && metadata[key].is_array()) { @@ -52,7 +52,8 @@ class FunctionMetadataTest : public ::testing::Test { } } std::sort(list.begin(), list.end(), [](const json& a, const json& b) { - return (a["outputType"] < b["outputType"]); + return folly::hasher()(a.dump()) < + folly::hasher()(b.dump()); }); } @@ -60,7 +61,7 @@ class FunctionMetadataTest : public ::testing::Test { const std::string& name, const std::string& expectedFile, size_t expectedSize) { - json metadataList = functionMetadata_.at(name); + json::array_t metadataList = functionMetadata_.at(name); EXPECT_EQ(metadataList.size(), expectedSize); std::string expectedStr = slurp( test::utils::getDataPath( @@ -68,7 +69,7 @@ class FunctionMetadataTest : public ::testing::Test { expectedFile)); auto expected = json::parse(expectedStr); - json expectedList = expected[name]; + json::array_t expectedList = expected[name]; sortMetadataList(expectedList); sortMetadataList(metadataList); for (auto i = 0; i < expectedSize; i++) { From 8586eb42098ea535a31b93a09abf196714422766 Mon Sep 17 00:00:00 2001 From: Ruth Kurniawati Date: Tue, 28 Oct 2025 17:19:41 -0400 Subject: [PATCH 5/7] Fix test failure due to an extraneous metadata Signed-off-by: Ruth Kurniawati --- .../main/functions/tests/data/Variance.json | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/functions/tests/data/Variance.json b/presto-native-execution/presto_cpp/main/functions/tests/data/Variance.json index 633d5d89e95a4..8bb21b981855f 100644 --- a/presto-native-execution/presto_cpp/main/functions/tests/data/Variance.json +++ b/presto-native-execution/presto_cpp/main/functions/tests/data/Variance.json @@ -104,27 +104,6 @@ "schema": "default", "typeVariableConstraints":[], "variableArity":false - }, - { - "aggregateMetadata": { - "intermediateType": "row(bigint,double,double)", - "isOrderSensitive": true - }, - "docString": "presto.default.variance", - "functionKind": "WINDOW", - "longVariableConstraints":[], - "outputType": "double", - "paramTypes": [ - "smallint" - ], - "routineCharacteristics": { - "determinism": "DETERMINISTIC", - "language": "CPP", - "nullCallClause": "CALLED_ON_NULL_INPUT" - }, - "schema": "default", - "typeVariableConstraints":[], - "variableArity":false } ] } From 50a1ebb6d52f635436c67adb52e59e4f938d5226 Mon Sep 17 00:00:00 2001 From: Ruth Kurniawati Date: Mon, 3 Nov 2025 19:54:14 -0500 Subject: [PATCH 6/7] Sort the function metadata by the function kind and parameter types Signed-off-by: Ruth Kurniawati --- .../main/functions/tests/FunctionMetadataTest.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp index d481f7bd26a40..65e03520f33fb 100644 --- a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp +++ b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp @@ -40,6 +40,16 @@ class FunctionMetadataTest : public ::testing::Test { functionMetadata_ = getFunctionsMetadata(); } + static std::string getFunctionKindAndParams(const json& entry) { + auto str = entry["functionKind"].get(); + if (entry["paramTypes"].is_array()) { + for (auto const& paramType : entry["paramTypes"]) { + str += "-" + paramType.get(); + } + } + return str; + } + static void sortMetadataList(json::array_t& list) { for (auto& metadata : list) { for (auto const& [key, val] : metadata.items()) { @@ -52,8 +62,8 @@ class FunctionMetadataTest : public ::testing::Test { } } std::sort(list.begin(), list.end(), [](const json& a, const json& b) { - return folly::hasher()(a.dump()) < - folly::hasher()(b.dump()); + return folly::hasher()(getFunctionKindAndParams(a)) < + folly::hasher()(getFunctionKindAndParams(b)); }); } From 8c705a2b2f13564fa80c6e91daf1e3db67afc012 Mon Sep 17 00:00:00 2001 From: Ruth Kurniawati Date: Wed, 5 Nov 2025 23:43:05 -0500 Subject: [PATCH 7/7] Use basic_json::dump to convert json::array_t to string, remove unnecessary function Signed-off-by: Ruth Kurniawati --- .../functions/tests/FunctionMetadataTest.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp index 65e03520f33fb..5a83dc75d8a06 100644 --- a/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp +++ b/presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp @@ -40,18 +40,9 @@ class FunctionMetadataTest : public ::testing::Test { functionMetadata_ = getFunctionsMetadata(); } - static std::string getFunctionKindAndParams(const json& entry) { - auto str = entry["functionKind"].get(); - if (entry["paramTypes"].is_array()) { - for (auto const& paramType : entry["paramTypes"]) { - str += "-" + paramType.get(); - } - } - return str; - } - - static void sortMetadataList(json::array_t& list) { + void sortMetadataList(json::array_t& list) { for (auto& metadata : list) { + // Sort constraint arrays for deterministic test comparisons. for (auto const& [key, val] : metadata.items()) { if (key.ends_with("Constraints") && metadata[key].is_array()) { std::sort( @@ -62,8 +53,10 @@ class FunctionMetadataTest : public ::testing::Test { } } std::sort(list.begin(), list.end(), [](const json& a, const json& b) { - return folly::hasher()(getFunctionKindAndParams(a)) < - folly::hasher()(getFunctionKindAndParams(b)); + return folly::hasher()( + a["functionKind"].dump() + a["paramTypes"].dump()) < + folly::hasher()( + b["functionKind"].dump() + b["paramTypes"].dump()); }); }