Skip to content

INT to date handling error #2709 - #2710

Merged
joocer merged 2 commits into
mainfrom
#2709
Jul 29, 2025
Merged

INT to date handling error #2709#2710
joocer merged 2 commits into
mainfrom
#2709

Conversation

@joocer

@joocer joocer commented Jul 29, 2025

Copy link
Copy Markdown
Member

Thank you for opening a Pull Request!

We appreciate your contribution to Opteryx. Your time and effort make a difference, and we鈥檙e excited to review your changes. To help ensure a smooth review process, please check the following:

Checklist for a Successful PR

  • Start the conversation: If you haven鈥檛 already, raise a bug/feature request or start a discussion. This ensures alignment on the change and approach.
  • Run the tests: Confirm that all tests pass without errors.
  • Maintain code coverage: If you鈥檝e added or modified source code ensure new tests are added to the test suite.
  • Update documentation and tests (if applicable): If your changes impact functionality, make sure the relevant docs and test cases are updated.

Fixes: #2709

Please replace <issue_number_goes_here> with the corresponding issue number.


Thank you for contributing to Opteryx! 馃帀

@joocer
joocer requested a review from Copilot July 29, 2025 22:14
@github-actions

Copy link
Copy Markdown

馃摝 Opteryx build version: 0.25.0-beta.1404

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request addresses issue #2709 related to INT to date handling error by improving type conversion and error handling in the date functions and filter operations.

Key changes:

  • Enhanced INT to datetime conversion to handle PyArrow data types
  • Improved filter node execution with better type checking and simplified logic
  • Version bump to 0.25.0-beta

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
opteryx/functions/date_functions.py Added support for PyArrow ChunkedArray and Array types in INT64 to datetime conversion
opteryx/operators/filter_node.py Simplified filter logic, improved type checking, and removed redundant error handling
opteryx/version.py Updated version to 0.25.0-beta and incremented build number

Comment on lines +66 to +67
mask = pyarrow.array(mask, type=pyarrow.bool_())

Copilot AI Jul 29, 2025

Copy link

Choose a reason for hiding this comment

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

The removal of exception handling around the mask conversion could cause unhandled exceptions. Consider adding back error handling with a more specific error message to help users understand what went wrong during mask conversion.

Copilot uses AI. Check for mistakes.
# Ensure mask is a BooleanArray
if not isinstance(mask, pyarrow.BooleanArray):
mask = pyarrow.array(mask, type=pyarrow.bool_())

Copilot AI Jul 29, 2025

Copy link

Choose a reason for hiding this comment

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

The condition if indices.size > 0: is insufficient. The original code also checked not numpy.all(mask is None) which prevented issues when the mask contains None values. Removing this check could lead to unexpected behavior when dealing with null values in the filter condition.

Suggested change
# Check if mask contains only None values
if numpy.all(mask is None):
yield morsel.slice(0, 0)
return

Copilot uses AI. Check for mistakes.
@sonarqubecloud

Copy link
Copy Markdown

Please retry analysis of this Pull-Request directly on SonarQube Cloud

@github-actions

Copy link
Copy Markdown

馃摝 Opteryx build version: 0.25.0-beta.1404

@joocer
joocer merged commit ae60ad7 into main Jul 29, 2025
22 checks passed
@joocer
joocer deleted the #2709 branch July 29, 2025 22:26
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.

clickbench tests no longer execute

2 participants