Skip to content

Commit de0fe1c

Browse files
[native] Pass extra_credential into QueryConfig (prestodb#26058)
## Description Some UDFs need to access credentials for authorization. Pass extra_credential into QueryConfig to support credentials access in both VectorFunction and SimpleFunction. Add the temporary overload that builds a QueryConfig from session properties and extraCredentials, including all extraCredentials so they can be consumed by UDFs and connectors. This implementation is a temporary solution until a more unified configuration mechanism (TokenProvider) is available. ## Motivation and Context ## Impact ## Test Plan Tested on verifier clusters ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [x] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ``` Co-authored-by: Vivian Chiang <[email protected]>
1 parent 18b7e33 commit de0fe1c

File tree

4 files changed

+119
-26
lines changed

4 files changed

+119
-26
lines changed

presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ void updateFromSystemConfigs(
180180
}
181181
} // namespace
182182

183-
velox::core::QueryConfig toVeloxConfigs(
183+
std::unordered_map<std::string, std::string> toVeloxConfigs(
184184
const protocol::SessionRepresentation& session) {
185185
std::unordered_map<std::string, std::string> configs;
186186

@@ -192,6 +192,23 @@ velox::core::QueryConfig toVeloxConfigs(
192192

193193
// Finally apply special case configs.
194194
updateVeloxConfigsWithSpecialCases(configs);
195+
return configs;
196+
}
197+
198+
velox::core::QueryConfig toVeloxConfigs(
199+
const protocol::SessionRepresentation& session,
200+
const std::map<std::string, std::string>& extraCredentials) {
201+
// Start with the session-based configuration
202+
auto configs = toVeloxConfigs(session);
203+
204+
// If there are any extra credentials, add them all to the config
205+
if (!extraCredentials.empty()) {
206+
// Create new config map with all extra credentials added
207+
configs.insert(
208+
extraCredentials.begin(),
209+
extraCredentials.end());
210+
}
211+
195212
return velox::core::QueryConfig(configs);
196213
}
197214

presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,21 @@ class QueryConfig;
2525

2626
namespace facebook::presto {
2727

28-
/// Translates Presto configs to Velox 'QueryConfig'. Presto query session
29-
/// properties take precedence over Presto system config properties.
30-
velox::core::QueryConfig toVeloxConfigs(
28+
/// Translates Presto configs to Velox 'QueryConfig' config map. Presto query
29+
/// session properties take precedence over Presto system config properties.
30+
std::unordered_map<std::string, std::string> toVeloxConfigs(
3131
const protocol::SessionRepresentation& session);
3232

33+
/// Translates Presto configs to Velox 'QueryConfig' config map. It is the
34+
/// temporary overload that builds a QueryConfig from session properties and
35+
/// extraCredentials, including all extraCredentials so they can be consumed by
36+
/// UDFs and connectors.
37+
/// This implementation is a temporary solution until a more unified
38+
/// configuration mechanism (TokenProvider) is available.
39+
velox::core::QueryConfig toVeloxConfigs(
40+
const protocol::SessionRepresentation& session,
41+
const std::map<std::string, std::string>& extraCredentials);
42+
3343
std::unordered_map<std::string, std::shared_ptr<velox::config::ConfigBase>>
3444
toConnectorConfigs(const protocol::TaskUpdateRequest& taskUpdateRequest);
3545

presto-native-execution/presto_cpp/main/QueryContextManager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ QueryContextManager::findOrCreateQueryCtx(
4545
const protocol::TaskUpdateRequest& taskUpdateRequest) {
4646
return findOrCreateQueryCtx(
4747
taskId,
48-
toVeloxConfigs(taskUpdateRequest.session),
48+
toVeloxConfigs(taskUpdateRequest.session, taskUpdateRequest.extraCredentials),
4949
toConnectorConfigs(taskUpdateRequest));
5050
}
5151

presto-native-execution/presto_cpp/main/tests/PrestoToVeloxQueryConfigTest.cpp

Lines changed: 87 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,12 @@
1717
#include "presto_cpp/main/SessionProperties.h"
1818
#include "presto_cpp/main/common/Configs.h"
1919
#include "presto_cpp/presto_protocol/core/presto_protocol_core.h"
20-
#include "velox/common/compression/Compression.h"
2120
#include "velox/core/QueryConfig.h"
22-
#include "velox/type/tz/TimeZoneMap.h"
2321

2422
using namespace facebook::presto;
2523
using namespace facebook::presto::protocol;
2624
using namespace facebook::velox;
25+
using namespace facebook::velox::core;
2726
class PrestoToVeloxQueryConfigTest : public testing::Test {
2827
protected:
2928
void SetUp() override {}
@@ -234,15 +233,15 @@ TEST_F(PrestoToVeloxQueryConfigTest, sessionPropertiesOverrideSystemConfigs) {
234233
session.systemProperties[testCase.sessionPropertyKey.value()] =
235234
testCase.sessionValue;
236235

237-
auto veloxConfig1 = toVeloxConfigs(session);
236+
auto veloxConfig1 = QueryConfig(toVeloxConfigs(session));
238237
testCase.validator(veloxConfig1, testCase.sessionValue);
239238

240239
// Test 2: Change session property to different value to ensure it's being
241240
// used
242241
session.systemProperties[testCase.sessionPropertyKey.value()] =
243242
testCase.differentSessionValue;
244243

245-
auto veloxConfig2 = toVeloxConfigs(session);
244+
auto veloxConfig2 = QueryConfig(toVeloxConfigs(session));
246245
testCase.validator(veloxConfig2, testCase.differentSessionValue);
247246

248247
// Test 3: Remove session property to test system config fallback
@@ -253,7 +252,7 @@ TEST_F(PrestoToVeloxQueryConfigTest, sessionPropertiesOverrideSystemConfigs) {
253252
}
254253

255254
// Test system config fallback behavior (applies to all test cases)
256-
auto veloxConfig3 = toVeloxConfigs(session);
255+
auto veloxConfig3 = QueryConfig(toVeloxConfigs(session));
257256

258257
// Get the actual system config default value using optionalProperty()
259258
auto* systemConfig = SystemConfig::instance();
@@ -280,7 +279,7 @@ TEST_F(PrestoToVeloxQueryConfigTest, sessionPropertiesOverrideSystemConfigs) {
280279
}
281280
}
282281

283-
auto veloxConfigAll = toVeloxConfigs(session);
282+
auto veloxConfigAll = QueryConfig(toVeloxConfigs(session));
284283

285284
// Verify all session properties are applied correctly
286285
for (const auto& testCase : testCases) {
@@ -300,7 +299,7 @@ TEST_F(PrestoToVeloxQueryConfigTest, queryTracingConfiguration) {
300299
session.systemProperties[SessionProperties::kQueryTraceMaxBytes] =
301300
"1048576"; // 1MB
302301

303-
auto veloxConfig = toVeloxConfigs(session);
302+
auto veloxConfig = QueryConfig(toVeloxConfigs(session));
304303

305304
EXPECT_TRUE(veloxConfig.queryTraceEnabled());
306305
EXPECT_EQ("/tmp/trace", veloxConfig.queryTraceDir());
@@ -313,27 +312,27 @@ TEST_F(PrestoToVeloxQueryConfigTest, queryTracingConfiguration) {
313312
"frag_123";
314313
session.systemProperties[SessionProperties::kQueryTraceShardId] = "shard_456";
315314

316-
auto veloxConfig1 = toVeloxConfigs(session);
315+
auto veloxConfig1 = QueryConfig(toVeloxConfigs(session));
317316
EXPECT_EQ(
318317
".*\\.frag_123\\..*\\.shard_456\\..*",
319318
veloxConfig1.queryTraceTaskRegExp());
320319

321320
// Test 3: Query tracing regex with only fragment ID
322321
session.systemProperties.erase(SessionProperties::kQueryTraceShardId);
323-
auto veloxConfig2 = toVeloxConfigs(session);
322+
auto veloxConfig2 = QueryConfig(toVeloxConfigs(session));
324323
EXPECT_EQ(
325324
".*\\.frag_123\\..*\\..*\\..*", veloxConfig2.queryTraceTaskRegExp());
326325

327326
// Test 4: Query tracing regex with only shard ID
328327
session.systemProperties.erase(SessionProperties::kQueryTraceFragmentId);
329328
session.systemProperties[SessionProperties::kQueryTraceShardId] = "shard_789";
330-
auto veloxConfig3 = toVeloxConfigs(session);
329+
auto veloxConfig3 = QueryConfig(toVeloxConfigs(session));
331330
EXPECT_EQ(
332331
".*\\..*\\..*\\.shard_789\\..*", veloxConfig3.queryTraceTaskRegExp());
333332

334333
// Test 5: Query tracing regex with neither fragment nor shard ID
335334
session.systemProperties.clear();
336-
auto veloxConfig4 = toVeloxConfigs(session);
335+
auto veloxConfig4 = QueryConfig(toVeloxConfigs(session));
337336
// When neither fragment nor shard ID is set, no regex should be constructed
338337
// The queryTraceTaskRegExp should be empty or default
339338
EXPECT_TRUE(
@@ -353,7 +352,7 @@ TEST_F(PrestoToVeloxQueryConfigTest, queryTracingConfiguration) {
353352
"fragment_789";
354353
session.systemProperties[SessionProperties::kQueryTraceShardId] = "shard_012";
355354

356-
auto veloxConfigComprehensive = toVeloxConfigs(session);
355+
auto veloxConfigComprehensive = QueryConfig(toVeloxConfigs(session));
357356

358357
// Verify all tracing properties are set correctly
359358
EXPECT_TRUE(veloxConfigComprehensive.queryTraceEnabled());
@@ -372,7 +371,7 @@ TEST_F(PrestoToVeloxQueryConfigTest, queryTracingConfiguration) {
372371
session.systemProperties[SessionProperties::kQueryTraceNodeId] =
373372
"disabled_node";
374373

375-
auto veloxConfigDisabled = toVeloxConfigs(session);
374+
auto veloxConfigDisabled = QueryConfig(toVeloxConfigs(session));
376375
EXPECT_FALSE(veloxConfigDisabled.queryTraceEnabled());
377376
// Even when disabled, other properties should still be set if provided
378377
EXPECT_EQ("/should/not/matter", veloxConfigDisabled.queryTraceDir());
@@ -385,16 +384,16 @@ TEST_F(PrestoToVeloxQueryConfigTest, shuffleCompressionHandling) {
385384
// Test various compression types
386385
session.systemProperties[SessionProperties::kShuffleCompressionCodec] =
387386
"ZSTD";
388-
auto veloxConfig1 = toVeloxConfigs(session);
387+
auto veloxConfig1 = QueryConfig(toVeloxConfigs(session));
389388
EXPECT_EQ("zstd", veloxConfig1.shuffleCompressionKind());
390389

391390
session.systemProperties[SessionProperties::kShuffleCompressionCodec] = "lz4";
392-
auto veloxConfig2 = toVeloxConfigs(session);
391+
auto veloxConfig2 = QueryConfig(toVeloxConfigs(session));
393392
EXPECT_EQ("lz4", veloxConfig2.shuffleCompressionKind());
394393

395394
session.systemProperties[SessionProperties::kShuffleCompressionCodec] =
396395
"none";
397-
auto veloxConfig3 = toVeloxConfigs(session);
396+
auto veloxConfig3 = QueryConfig(toVeloxConfigs(session));
398397
EXPECT_EQ("none", veloxConfig3.shuffleCompressionKind());
399398
}
400399

@@ -445,25 +444,92 @@ TEST_F(PrestoToVeloxQueryConfigTest, specialHardCodedPrestoConfigurations) {
445444

446445
session.systemProperties.clear();
447446
session.systemProperties[SessionProperties::kLegacyTimestamp] = "true";
448-
auto veloxConfig1 = toVeloxConfigs(session);
447+
auto veloxConfig1 = QueryConfig(toVeloxConfigs(session));
449448
EXPECT_TRUE(veloxConfig1.adjustTimestampToTimezone());
450449

451450
session.systemProperties.clear();
452451
session.systemProperties[SessionProperties::kLegacyTimestamp] = "false";
453-
auto veloxConfig2 = toVeloxConfigs(session);
452+
auto veloxConfig2 = QueryConfig(toVeloxConfigs(session));
454453
EXPECT_FALSE(veloxConfig2.adjustTimestampToTimezone());
455454

456455
session.systemProperties.clear();
457-
auto veloxConfig3 = toVeloxConfigs(session);
456+
auto veloxConfig3 = QueryConfig(toVeloxConfigs(session));
458457
EXPECT_TRUE(veloxConfig3.adjustTimestampToTimezone());
459458

460459
session.systemProperties.clear();
461-
auto veloxConfig8 = toVeloxConfigs(session);
460+
auto veloxConfig8 = QueryConfig(toVeloxConfigs(session));
462461
EXPECT_EQ(1000, veloxConfig8.driverCpuTimeSliceLimitMs());
463462

464463
session.systemProperties.clear();
465464
session.systemProperties[SessionProperties::kDriverCpuTimeSliceLimitMs] =
466465
"2000";
467-
auto veloxConfig9 = toVeloxConfigs(session);
466+
auto veloxConfig9 = QueryConfig(toVeloxConfigs(session));
468467
EXPECT_EQ(2000, veloxConfig9.driverCpuTimeSliceLimitMs());
469468
}
469+
470+
TEST_F(PrestoToVeloxQueryConfigTest, sessionAndExtraCredentialsOverload) {
471+
// --- Test 1: Basic session with empty extra credentials ---
472+
{
473+
auto session = createBasicSession();
474+
475+
std::map<std::string, std::string> emptyCredentials;
476+
auto veloxConfig = toVeloxConfigs(session, emptyCredentials);
477+
478+
// No unexpected credentials should appear.
479+
// Get raw configs to verify.
480+
auto raw = veloxConfig.rawConfigsCopy();
481+
EXPECT_EQ(0, raw.count("cat_token"));
482+
EXPECT_EQ(0, raw.count("auth_header"));
483+
EXPECT_EQ(0, raw.count("custom_credential"));
484+
}
485+
486+
// --- Test 2: Session with extra credentials (CAT, auth header, custom) ---
487+
{
488+
auto session = createBasicSession();
489+
490+
std::map<std::string, std::string> extraCredentials{
491+
{"cat_token", "test_cat_token_value"},
492+
{"auth_header", "Bearer xyz123"},
493+
{"custom_credential", "custom_value"},
494+
};
495+
496+
auto veloxConfig = toVeloxConfigs(session, extraCredentials);
497+
// Get raw configs to verify.
498+
auto raw = veloxConfig.rawConfigsCopy();
499+
500+
// Extra credentials included in the raw config.
501+
ASSERT_TRUE(raw.count("cat_token"));
502+
ASSERT_TRUE(raw.count("auth_header"));
503+
ASSERT_TRUE(raw.count("custom_credential"));
504+
EXPECT_EQ("test_cat_token_value", raw.at("cat_token"));
505+
EXPECT_EQ("Bearer xyz123", raw.at("auth_header"));
506+
EXPECT_EQ("custom_value", raw.at("custom_credential"));
507+
}
508+
509+
// --- Test 3: Merge behavior: session system properties + more credentials ---
510+
{
511+
auto session = createBasicSession();
512+
// Verify that typed options reflect session settings.
513+
session.systemProperties[core::QueryConfig::kSpillEnabled] = "true";
514+
session.systemProperties[SessionProperties::kJoinSpillEnabled] = "false";
515+
516+
std::map<std::string, std::string> moreCredentials{
517+
{"isolation_domain_token", "ids_token_abc123"},
518+
{"verification_key", "verify_key_xyz"},
519+
};
520+
521+
auto veloxConfig = toVeloxConfigs(session, moreCredentials);
522+
523+
// Typed properties should be applied from the session.
524+
EXPECT_TRUE(veloxConfig.spillEnabled());
525+
EXPECT_FALSE(veloxConfig.joinSpillEnabled());
526+
527+
// Extra credentials should be present in the raw map.
528+
// Get raw configs to verify.
529+
auto raw = veloxConfig.rawConfigsCopy();
530+
ASSERT_TRUE(raw.count("isolation_domain_token"));
531+
ASSERT_TRUE(raw.count("verification_key"));
532+
EXPECT_EQ("ids_token_abc123", raw.at("isolation_domain_token"));
533+
EXPECT_EQ("verify_key_xyz", raw.at("verification_key"));
534+
}
535+
}

0 commit comments

Comments
 (0)