Skip to content

GH-50078: [C++][ORC] Avoid signed overflow when converting timestamps#50035

Open
jmestwa-coder wants to merge 2 commits into
apache:mainfrom
jmestwa-coder:orc-timestamp-nanos-overflow
Open

GH-50078: [C++][ORC] Avoid signed overflow when converting timestamps#50035
jmestwa-coder wants to merge 2 commits into
apache:mainfrom
jmestwa-coder:orc-timestamp-nanos-overflow

Conversation

@jmestwa-coder
Copy link
Copy Markdown

@jmestwa-coder jmestwa-coder commented May 25, 2026

Rationale for this change

A far-future ORC timestamp (after ~2262) makes AppendTimestampBatch in cpp/src/arrow/adapters/orc/util.cc overflow int64 nanoseconds in seconds * kOneSecondNanos + nanos. Reducing the multiply under -fsanitize=signed-integer-overflow:

runtime error: signed integer overflow:
  10000000000 * 1000000000 cannot be represented in type 'int64_t'

What changes are included in this PR?

Detect the overflow with MultiplyWithOverflow/AddWithOverflow and return Status::Invalid for out-of-range values instead of computing the product with undefined behavior. Null slots are skipped.

Are these changes tested?

Verified the offending expression with a standalone -fsanitize=signed-integer-overflow reproducer and confirmed the patched path returns an error rather than overflowing.

Are there any user-facing changes?

No.

@kou
Copy link
Copy Markdown
Member

kou commented May 25, 2026

@wgtmac
Copy link
Copy Markdown
Member

wgtmac commented May 26, 2026

I agree with @kou. Changes like this require an issue. The change itself looks good.

@jmestwa-coder jmestwa-coder changed the title MINOR: [C++][ORC] Avoid signed overflow when converting timestamps GH-50078: [C++][ORC] Avoid signed overflow when converting timestamps Jun 2, 2026
@jmestwa-coder
Copy link
Copy Markdown
Author

Opened #50078 for this and retitled the PR to reference it. Also restored the PR template. Thanks both.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

⚠️ GitHub issue #50078 has been automatically assigned in GitHub to PR creator.

@wgtmac
Copy link
Copy Markdown
Member

wgtmac commented Jun 2, 2026

Thanks for updating it! Is it possible to add a test case?

@jmestwa-coder
Copy link
Copy Markdown
Author

Added one in adapter_test.cc (TimestampOutOfRangeIsRejected) - it feeds a far-future ORC timestamp (10000000000s, ~year 2286) through AppendBatch and asserts it returns Invalid instead of overflowing.

Copy link
Copy Markdown
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @jmestwa-coder!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants