Skip to content

fix array_repeat above Presto repeat limit - #797

Open
JinyuanZhang617 wants to merge 1 commit into
bytedance:mainfrom
JinyuanZhang617:enlarge_array_repeat_github_main
Open

fix array_repeat above Presto repeat limit#797
JinyuanZhang617 wants to merge 1 commit into
bytedance:mainfrom
JinyuanZhang617:enlarge_array_repeat_github_main

Conversation

@JinyuanZhang617

@JinyuanZhang617 JinyuanZhang617 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: #652

This PR fixes a compatibility issue in the Spark-compatible array_repeat implementation.

Previously, repeat_allow_negative_count, which is used to provide Spark-compatible behavior, reused the same 10,000-result-entry-per-row limit as Presto repeat. However, Spark array_repeat does not enforce this Presto-specific limit. As a result, valid Spark queries such as repeating an element more than 10,000 times could fail unexpectedly.

This PR separates the Presto repeat behavior from the Spark-compatible repeat_allow_negative_count behavior:

  • Presto repeat still enforces the 10,000 entries per row limit.
  • Spark-compatible repeat_allow_negative_count allows counts greater than 10,000.
  • A total result size check is still kept to prevent exceeding the maximum vector size.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 🚀 Performance improvement (optimization)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • 🔨 Refactoring (no logic changes)
  • 🔧 Build/CI or Infrastructure changes
  • 📝 Documentation only

Description

Describe your changes in detail.
For complex logic, explain the "Why" and "How".

This PR updates the implementation of the repeat array function to distinguish between Presto-compatible and Spark-compatible semantics.

Previously, RepeatFunction::checkCount always enforced kMaxResultEntries = 10,000, regardless of whether the function was used for Presto repeat or Spark-compatible repeat_allow_negative_count.

The change introduces a new enforceMaxResultEntries flag in RepeatFunction:

  • makeRepeat creates RepeatFunction(false, true), so Presto repeat keeps enforcing the 10,000 limit.
  • makeRepeatAllowNegativeCount creates RepeatFunction(true, false), so Spark-compatible repeat_allow_negative_count allows counts greater than 10,000.

In addition, this PR adds checkTotalCount to validate the total number of output entries before allocating result buffers. This prevents the total repeated result size from exceeding the maximum supported vector_size_t size.

The implementation also changes the accumulated total count in the non-constant count path from int32_t to int64_t, avoiding integer overflow when summing large repeat counts across rows.

A new unit test is added to verify that repeat_allow_negative_count works when the repeat count is above the Presto limit. The test covers both:

  • non-constant count input
  • constant count input

Performance Impact

  • No Impact: This is a correctness fix for function semantics. It only adds a configuration flag and a total-size safety check, and does not introduce algorithmic changes to the repeat execution path.

  • Positive Impact: I have run benchmarks.

    Click to view Benchmark Results
    N/A
    
  • Negative Impact: Explained below (e.g., trade-off for correctness).

Release Note

Release Note:
- Fixed Spark-compatible array_repeat behavior to allow repeat counts greater than the Presto repeat limit of 10,000.
- Added safety validation for total repeat result size to avoid exceeding the maximum vector size.

Checklist (For Author)

  • I have added/updated unit tests (ctest).

Breaking Changes

  • No

@JinyuanZhang617
JinyuanZhang617 force-pushed the enlarge_array_repeat_github_main branch from 4461e72 to 4b866b0 Compare July 28, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant