From b8631ae7141c948c4d40b5c38190ef6b1cb7b723 Mon Sep 17 00:00:00 2001
From: Mahadevuni Naveen Kumar <nmahadevuni@gmail.com>
Date: Thu, 4 Dec 2025 18:17:49 +0530
Subject: [PATCH] refactor(native): Fail on empty or non-existent config
 environment variable

---
 .../presto_cpp/main/common/ConfigReader.cpp   | 12 +++----
 .../main/common/tests/ConfigTest.cpp          | 32 ++++++++++++++++---
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/presto-native-execution/presto_cpp/main/common/ConfigReader.cpp b/presto-native-execution/presto_cpp/main/common/ConfigReader.cpp
index b10d9962c0bb9..c34760b0db465 100644
--- a/presto-native-execution/presto_cpp/main/common/ConfigReader.cpp
+++ b/presto-native-execution/presto_cpp/main/common/ConfigReader.cpp
@@ -27,15 +27,13 @@ namespace {
 void extractValueIfEnvironmentVariable(std::string& value) {
   if (value.size() > 3 && value.substr(0, 2) == "${" && value.back() == '}') {
     auto envName = value.substr(2, value.size() - 3);
-
     const char* envVal = std::getenv(envName.c_str());
-    if (envVal != nullptr) {
-      if (strlen(envVal) == 0) {
-        LOG(WARNING) << fmt::format(
-            "Config environment variable {} is empty.", envName);
-      }
-      value = std::string(envVal);
+
+    if (envVal == nullptr || strlen(envVal) == 0) {
+      VELOX_USER_FAIL(
+          "Config environment variable {} doesn't exist or is empty.", envName);
     }
+    value = std::string(envVal);
   }
 }
 } // namespace
diff --git a/presto-native-execution/presto_cpp/main/common/tests/ConfigTest.cpp b/presto-native-execution/presto_cpp/main/common/tests/ConfigTest.cpp
index ce07dc3cfa545..00a73df84eff8 100644
--- a/presto-native-execution/presto_cpp/main/common/tests/ConfigTest.cpp
+++ b/presto-native-execution/presto_cpp/main/common/tests/ConfigTest.cpp
@@ -319,6 +319,8 @@ TEST_F(ConfigTest, optionalNodeId) {
 TEST_F(ConfigTest, readConfigEnvVarTest) {
   const std::string kEnvVarName = "PRESTO_READ_CONFIG_TEST_VAR";
   const std::string kEmptyEnvVarName = "PRESTO_READ_CONFIG_TEST_EMPTY_VAR";
+  const std::string kNonExistEnvVarName =
+      "PRESTO_READ_CONFIG_TEST_NON_EXIST_VAR";
 
   const std::string kPlainTextKey = "plain-text";
   const std::string kPlainTextValue = "plain-text-value";
@@ -337,11 +339,9 @@ TEST_F(ConfigTest, readConfigEnvVarTest) {
       fmt::format("{}=${{{}}}\n", kEnvVarKey, kEnvVarName) +
       fmt::format("{}=${{{}\n", kEnvVarKey2, kEnvVarName) +
       fmt::format("{}={}}}\n", kEnvVarKey3, kEnvVarName) +
-      fmt::format("{}=${{}}\n", kNoEnvVarKey) +
-      fmt::format("{}=${{{}}}\n", kEmptyEnvVarKey, kEmptyEnvVarName));
+      fmt::format("{}=${{}}\n", kNoEnvVarKey));
 
   setenv(kEnvVarName.c_str(), kEnvVarValue.c_str(), 1);
-  setenv(kEmptyEnvVarName.c_str(), "", 1);
 
   auto properties = presto::util::readConfig(configFilePath_);
   std::unordered_map<std::string, std::string> expected{
@@ -349,10 +349,32 @@ TEST_F(ConfigTest, readConfigEnvVarTest) {
       {kEnvVarKey, kEnvVarValue},
       {kEnvVarKey2, "${PRESTO_READ_CONFIG_TEST_VAR"},
       {kEnvVarKey3, "PRESTO_READ_CONFIG_TEST_VAR}"},
-      {kNoEnvVarKey, "${}"},
-      {kEmptyEnvVarKey, ""}};
+      {kNoEnvVarKey, "${}"}};
   ASSERT_EQ(properties, expected);
 
+  // Empty env var
+  auto testInvalidEnvVar = [this](
+                               const std::string& fileContent,
+                               const std::string& expectedErrorMsg) {
+    cleanupConfigFilePath();
+    setUpConfigFilePath();
+    writeConfigFile(fileContent);
+    VELOX_ASSERT_THROW(
+        presto::util::readConfig(configFilePath_), expectedErrorMsg);
+  };
+
+  setenv(kEmptyEnvVarName.c_str(), "", 1);
+  testInvalidEnvVar(
+      fmt::format("{}=${{{}}}\n", kEmptyEnvVarKey, kEmptyEnvVarName),
+      fmt::format(
+          "Config environment variable {} doesn't exist or is empty",
+          kEmptyEnvVarName));
+  testInvalidEnvVar(
+      fmt::format("{}=${{{}}}\n", kEmptyEnvVarKey, kNonExistEnvVarName),
+      fmt::format(
+          "Config environment variable {} doesn't exist or is empty",
+          kNonExistEnvVarName));
+
   unsetenv(kEnvVarName.c_str());
   unsetenv(kEmptyEnvVarName.c_str());
 }