[fix](compile) disambiguate Aws::Utils::DateTime ctor in kinesis_conf#63727
Merged
airborne12 merged 1 commit intoMay 27, 2026
Merged
Conversation
…sis_conf Aws::Utils::DateTime has both DateTime(int64_t) and DateTime(double) constructors. Passing the result of std::stol() makes overload resolution ambiguous on platforms where int64_t is typedef'd to long long (macOS ARM64), because long->long long and long->double are both same-rank conversions. Switching to std::stoll() would break the symmetric Linux x86_64 case where int64_t is long. Use std::stoll() to parse 64-bit explicitly, then static_cast<int64_t> to pick the integer constructor unambiguously on every platform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31378 ms |
Contributor
TPC-DS: Total hot run time: 171474 ms |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Member
Author
|
/review |
gavinchou
approved these changes
May 27, 2026
Contributor
There was a problem hiding this comment.
Review result: no blocking issues found. The change resolves the AWS SDK DateTime constructor ambiguity by explicitly selecting the int64_t overload after parsing a 64-bit value, and it preserves the existing request application/error propagation behavior.
Critical checkpoint conclusions:
- Goal and proof: The PR goal is a compile fix for ambiguous Aws::Utils::DateTime construction; the code accomplishes this directly. PR description reports a macOS ARM64 BE build verification, but I did not run a build in this review.
- Scope: The actual GitHub PR patch is small and focused on the single ambiguous constructor call.
- Concurrency: No new concurrency or shared mutable state is introduced.
- Lifecycle/static initialization: No lifecycle or static initialization behavior is changed.
- Configuration: No new configuration item is added; existing aws.kinesis.timestamp handling is unchanged except for constructor disambiguation.
- Compatibility/storage/transactions: No protocol, storage format, transaction, or persistence behavior is involved.
- Parallel paths: The modified path is the GetShardIterator timestamp application path; other Kinesis request paths do not construct Aws::Utils::DateTime.
- Error handling: Existing Status propagation is preserved; parse/apply failures remain reported as non-OK Status.
- Tests: No new test is required for this compile-only overload-resolution fix; existing runtime semantics are unchanged.
- Observability/performance: No observability change is needed; no meaningful performance impact.
User focus: No additional user-provided review focus was present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
be/src/load/routine_load/kinesis_conf.cpp:118constructsAws::Utils::DateTimefromstd::stol(...). The AWS SDK declares two constructors:On macOS ARM64 (Apple LP64 with libc++),
int64_tis typedef'd tolong long— distinct from thelongthatstd::stolreturns. Bothlong -> long longandlong -> doubleare same-rank conversions, so overload resolution is ambiguous and the build fails:Note: a naive switch to
std::stollwould symmetrically break Linux x86_64 (glibc) builds, whereint64_tis typedef'd tolong— there,long longis ambiguous against(long)vs(double)for the same reason.The portable fix is to parse with
std::stoll(explicit 64-bit, semantically correct for milliseconds-since-epoch) andstatic_cast<int64_t>to bind to the integer constructor regardless of whatint64_tresolves to on the target platform:request.SetTimestamp( Aws::Utils::DateTime(static_cast<int64_t>(std::stoll(it->second))));Verified with an overload-resolution mini-repro using the same clang-20 toolchain: only the explicit-cast form is unambiguous on both
int64_t = long long(macOS) andint64_t = long(Linux x86_64) targets.Release note
None
Check List (For Author)
Test
Manual test: verified
./build.sh --be -j 20now succeeds on macOS ARM64. The runtime value is identical (std::stollparses the same string to a 64-bit integer;static_cast<int64_t>is a no-op cast on every supported platform).Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)