Skip to content

Commit 92a6920

Browse files
authored
fix(native): Revert PR 26488 (#26508)
With #26488 All queries fails with ``` presto:di> select * from tpch.sf1.orders limit 2 Query 20251101_231501_00108_hcf8c failed: Non-whitespace character found after end of conversion: "MB" ``` The reason is the value requires unit like MB #26488 (comment) Revering to ensure main branch is stable and working conditiion. After revert ``` presto:di> select * from tpch.sf1.orders limit 2 Query 20251101_234105_00001_c8suz, FINISHED ``` ``` == NO RELEASE NOTE == ```
1 parent cc966ee commit 92a6920

File tree

4 files changed

+0
-120
lines changed

4 files changed

+0
-120
lines changed

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,6 @@ void updateFromSystemConfigs(
153153
{std::string{SystemConfig::kTaskPartitionedWriterCount},
154154
velox::core::QueryConfig::kTaskPartitionedWriterCount},
155155

156-
{std::string{SystemConfig::kExchangeMaxBufferSize},
157-
velox::core::QueryConfig::kMaxExchangeBufferSize},
158-
159156
{std::string(SystemConfig::kSinkMaxBufferSize),
160157
velox::core::QueryConfig::kMaxOutputBufferSize,
161158
[](const auto& value) {

presto-native-execution/presto_cpp/main/common/Configs.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ SystemConfig::SystemConfig() {
244244
BOOL_PROP(kExchangeEnableConnectionPool, true),
245245
BOOL_PROP(kExchangeEnableBufferCopy, true),
246246
BOOL_PROP(kExchangeImmediateBufferTransfer, true),
247-
NUM_PROP(kExchangeMaxBufferSize, 32UL << 20),
248247
NUM_PROP(kTaskRunTimeSliceMicros, 50'000),
249248
BOOL_PROP(kIncludeNodeInSpillPath, false),
250249
NUM_PROP(kOldTaskCleanUpMs, 60'000),
@@ -910,10 +909,6 @@ bool SystemConfig::exchangeImmediateBufferTransfer() const {
910909
return optionalProperty<bool>(kExchangeImmediateBufferTransfer).value();
911910
}
912911

913-
uint64_t SystemConfig::exchangeMaxBufferSize() const {
914-
return optionalProperty<uint64_t>(kExchangeMaxBufferSize).value();
915-
}
916-
917912
int32_t SystemConfig::taskRunTimeSliceMicros() const {
918913
return optionalProperty<int32_t>(kTaskRunTimeSliceMicros).value();
919914
}

presto-native-execution/presto_cpp/main/common/Configs.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -717,11 +717,6 @@ class SystemConfig : public ConfigBase {
717717
kExchangeHttpClientNumCpuThreadsHwMultiplier{
718718
"exchange.http-client.num-cpu-threads-hw-multiplier"};
719719

720-
/// Maximum size in bytes to accumulate in ExchangeQueue. Enforced
721-
/// approximately, not strictly.
722-
static constexpr std::string_view kExchangeMaxBufferSize{
723-
"exchange.max-buffer-size"};
724-
725720
/// The maximum timeslice for a task on thread if there are threads queued.
726721
static constexpr std::string_view kTaskRunTimeSliceMicros{
727722
"task-run-timeslice-micros"};
@@ -1083,8 +1078,6 @@ class SystemConfig : public ConfigBase {
10831078

10841079
bool exchangeImmediateBufferTransfer() const;
10851080

1086-
uint64_t exchangeMaxBufferSize() const;
1087-
10881081
int32_t taskRunTimeSliceMicros() const;
10891082

10901083
bool includeNodeInSpillPath() const;

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

Lines changed: 0 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -571,108 +571,3 @@ TEST_F(PrestoToVeloxQueryConfigTest, sessionStartTimeConfiguration) {
571571
EXPECT_EQ(
572572
std::numeric_limits<int64_t>::max(), veloxConfig5.sessionStartTimeMs());
573573
}
574-
575-
TEST_F(PrestoToVeloxQueryConfigTest, systemConfigsWithoutSessionOverride) {
576-
// Verifies system configs are properly applied when no session properties
577-
// override them. Uses exact count matching to catch any config additions or
578-
// removals.
579-
580-
auto session = createBasicSession();
581-
session.systemProperties.clear();
582-
auto veloxConfigs = toVeloxConfigs(session);
583-
584-
struct SystemConfigMapping {
585-
std::string veloxConfigKey;
586-
std::string systemConfigKey;
587-
};
588-
589-
// MUST match veloxToPrestoConfigMapping in PrestoToVeloxQueryConfig.cpp
590-
std::vector<SystemConfigMapping> expectedMappings = {
591-
{core::QueryConfig::kQueryMaxMemoryPerNode,
592-
std::string(SystemConfig::kQueryMaxMemoryPerNode)},
593-
{core::QueryConfig::kSpillFileCreateConfig,
594-
std::string(SystemConfig::kSpillerFileCreateConfig)},
595-
{core::QueryConfig::kSpillEnabled,
596-
std::string(SystemConfig::kSpillEnabled)},
597-
{core::QueryConfig::kJoinSpillEnabled,
598-
std::string(SystemConfig::kJoinSpillEnabled)},
599-
{core::QueryConfig::kOrderBySpillEnabled,
600-
std::string(SystemConfig::kOrderBySpillEnabled)},
601-
{core::QueryConfig::kAggregationSpillEnabled,
602-
std::string(SystemConfig::kAggregationSpillEnabled)},
603-
{core::QueryConfig::kRequestDataSizesMaxWaitSec,
604-
std::string(SystemConfig::kRequestDataSizesMaxWaitSec)},
605-
{core::QueryConfig::kMaxSplitPreloadPerDriver,
606-
std::string(SystemConfig::kDriverMaxSplitPreload)},
607-
{core::QueryConfig::kMaxLocalExchangePartitionBufferSize,
608-
std::string(SystemConfig::kMaxLocalExchangePartitionBufferSize)},
609-
{core::QueryConfig::kPrestoArrayAggIgnoreNulls,
610-
std::string(SystemConfig::kUseLegacyArrayAgg)},
611-
{core::QueryConfig::kTaskWriterCount,
612-
std::string(SystemConfig::kTaskWriterCount)},
613-
{core::QueryConfig::kTaskPartitionedWriterCount,
614-
std::string(SystemConfig::kTaskPartitionedWriterCount)},
615-
{core::QueryConfig::kMaxExchangeBufferSize,
616-
std::string(SystemConfig::kExchangeMaxBufferSize)},
617-
{core::QueryConfig::kMaxOutputBufferSize,
618-
std::string(SystemConfig::kSinkMaxBufferSize)},
619-
{core::QueryConfig::kMaxPartitionedOutputBufferSize,
620-
std::string(SystemConfig::kDriverMaxPagePartitioningBufferSize)},
621-
{core::QueryConfig::kMaxPartialAggregationMemory,
622-
std::string(SystemConfig::kTaskMaxPartialAggregationMemory)},
623-
};
624-
625-
const size_t kExpectedSystemConfigMappingCount = 16;
626-
EXPECT_EQ(kExpectedSystemConfigMappingCount, expectedMappings.size())
627-
<< "Update expectedMappings to match veloxToPrestoConfigMapping";
628-
629-
// Verify each system config mapping is present when it has a value
630-
auto* systemConfig = SystemConfig::instance();
631-
for (const auto& mapping : expectedMappings) {
632-
auto systemValue = systemConfig->optionalProperty(mapping.systemConfigKey);
633-
if (systemValue.hasValue()) {
634-
EXPECT_TRUE(veloxConfigs.count(mapping.veloxConfigKey) > 0)
635-
<< "Expected '" << mapping.veloxConfigKey << "' when system config '"
636-
<< mapping.systemConfigKey << "' = " << systemValue.value();
637-
}
638-
}
639-
640-
// Verify special case configs (always added)
641-
EXPECT_TRUE(
642-
veloxConfigs.count(core::QueryConfig::kAdjustTimestampToTimezone) > 0);
643-
EXPECT_EQ(
644-
"true", veloxConfigs.at(core::QueryConfig::kAdjustTimestampToTimezone));
645-
646-
EXPECT_TRUE(
647-
veloxConfigs.count(core::QueryConfig::kDriverCpuTimeSliceLimitMs) > 0);
648-
EXPECT_EQ(
649-
"1000", veloxConfigs.at(core::QueryConfig::kDriverCpuTimeSliceLimitMs));
650-
651-
// Verify session-specific configs
652-
EXPECT_TRUE(veloxConfigs.count(core::QueryConfig::kSessionStartTime) > 0);
653-
EXPECT_EQ(
654-
"1234567890", veloxConfigs.at(core::QueryConfig::kSessionStartTime));
655-
656-
// Calculate expected exact count
657-
size_t expectedExactConfigs = 0;
658-
for (const auto& mapping : expectedMappings) {
659-
if (systemConfig->optionalProperty(mapping.systemConfigKey).hasValue()) {
660-
expectedExactConfigs++;
661-
}
662-
}
663-
expectedExactConfigs += 2; // kAdjustTimestampToTimezone,
664-
// kDriverCpuTimeSliceLimitMs
665-
expectedExactConfigs += 1; // kSessionStartTime
666-
667-
// Use exact matching to catch any config additions/removals
668-
EXPECT_EQ(veloxConfigs.size(), expectedExactConfigs)
669-
<< "Config count mismatch indicates mapping change. Expected "
670-
<< expectedExactConfigs << ", got " << veloxConfigs.size();
671-
672-
// Debug output
673-
std::cout << "System configs (no session overrides):" << std::endl;
674-
for (const auto& [key, value] : veloxConfigs) {
675-
std::cout << " " << key << " = " << value << std::endl;
676-
}
677-
std::cout << "Total: " << veloxConfigs.size() << std::endl;
678-
}

0 commit comments

Comments
 (0)