Skip to content

Conversation

Copy link

Copilot AI commented Nov 6, 2025

Velox's Iceberg bucket function used bucket(numBuckets, input) while Presto PR #25951 and SQL engines (Trino, Presto) use bucket(value, numBuckets). This change swaps the parameter order to match SQL conventions.

Changes

  • Function signature: Swapped parameter order in all BucketFunction and BucketDecimalFunction template methods from call(out, numBuckets, input) to call(out, input, numBuckets)
  • Registration: Updated registerBucketFunctions() to register types in new order: <ReturnType, InputType, int32_t> instead of <ReturnType, int32_t, InputType>
  • Tests: Updated all test cases and helper functions to pass parameters in new order
  • Documentation: Updated RST docs with new signature and examples

Example

-- Old signature
SELECT bucket(128, 'abcd');  -- Returns 4

-- New signature (matches Presto/Trino)
SELECT bucket('abcd', 128);  -- Returns 4

Affects all supported types: INTEGER, BIGINT, DECIMAL, DATE, TIMESTAMP, VARCHAR, VARBINARY.

Original prompt

is facebookincubator#13174 and prestodb/presto#25951 same? I need to implement the prestodb/presto#25951 in velox


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Implement Presto PR 25951 in Velox Align Iceberg bucket function parameter order with Presto/SQL convention Nov 6, 2025
Copilot AI requested a review from Joe-Abraham November 6, 2025 08:29
Copilot finished work on behalf of Joe-Abraham November 6, 2025 08:29
@Joe-Abraham Joe-Abraham closed this Nov 6, 2025
@Joe-Abraham Joe-Abraham deleted the copilot/implement-presto-pull-25951 branch November 6, 2025 08:32
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.

2 participants