Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YQL-18435: Clear the error in Python numeric casts #4699

Merged

Conversation

igormunkin
Copy link
Collaborator

@igormunkin igormunkin commented May 20, 2024

The error indicator raised by the Python runtime, when the particular numeric cast fails, should be reset, when the exception is considered "handled" (e.g. rethrow of the unsuccessful cast error). PyErr_Clear had been lost since the very first commit, related to the Python bindings, i.e. 93acd0b ("Move YQL python UDFs in OS (#4139)"), though the similar exceptions are handled right in the "test" helpers (see TRY_FROM_PYTHON_FLOAT and TRY_FROM_PYTHON_LONG).

Considering the changes related to the Python integral types between 2.x and 3.x major releases, it's enough (almost) to provide only the one test for PyObject-to-int/long conversion.

Follows up #4139

Changelog category

  • Bugfix

Copy link

github-actions bot commented May 20, 2024

2024-05-20 18:49:56 UTC Pre-commit check for f5801e9 has started.
2024-05-20 18:49:59 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-20 18:52:22 UTC Build successful.

Copy link

github-actions bot commented May 20, 2024

2024-05-20 18:50:00 UTC Pre-commit check for f5801e9 has started.
2024-05-20 18:50:03 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-20 18:52:33 UTC Build successful.
2024-05-20 18:54:01 UTC Tests are running...
🟢 2024-05-20 18:54:41 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
164 155 0 0 9 0

Copy link

github-actions bot commented May 20, 2024

2024-05-20 18:50:08 UTC Pre-commit check for f5801e9 has started.
2024-05-20 18:50:10 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-20 18:52:27 UTC Build successful.
2024-05-20 18:53:52 UTC Tests are running...
🟢 2024-05-20 18:54:36 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
165 156 0 0 9 0

The error indicator raised by the Python runtime, when the particular
numeric cast fails, should be reset, when the exception is considered
"handled" (e.g. rethrow of the unsuccessful cast error). <PyErr_Clear>
had been lost since the very first commit, related to the Python
bindings, i.e. 93acd0b ("Move YQL python UDFs in OS (ydb-platform#4139)"),
though the similar exceptions are handled right in the "test" helpers
(see <TRY_FROM_PYTHON_FLOAT> and <TRY_FROM_PYTHON_LONG>).

Considering the changes related to the Python integral types between 2.x
and 3.x major releases, it's enough (almost) to provide only the one
test for PyObject-to-int/long conversion.

Follows up ydb-platform#4139
@igormunkin igormunkin force-pushed the YQL-18435-fix-pyerr-rethrow-misuse branch from 9000213 to 97bdf8a Compare May 20, 2024 20:38
@igormunkin igormunkin changed the title YQL-18435: Fix PyErr rethrow misuse YQL-18435: Clear the error in Python numeric casts May 20, 2024
Copy link

github-actions bot commented May 20, 2024

2024-05-20 20:42:02 UTC Pre-commit check for 1cfaa62 has started.
2024-05-20 20:42:04 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-20 20:43:31 UTC Build successful.
2024-05-20 20:44:58 UTC Tests are running...
🟢 2024-05-20 20:45:37 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
166 157 0 0 9 0

Copy link

github-actions bot commented May 20, 2024

2024-05-20 20:42:08 UTC Pre-commit check for 1cfaa62 has started.
2024-05-20 20:42:11 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-20 20:45:19 UTC Build successful.

Copy link

github-actions bot commented May 20, 2024

2024-05-20 20:42:18 UTC Pre-commit check for 1cfaa62 has started.
2024-05-20 20:42:20 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-20 20:43:47 UTC Build successful.
2024-05-20 20:45:12 UTC Tests are running...
🟢 2024-05-20 20:46:03 UTC Tests successful.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
167 158 0 0 9 0

@igormunkin igormunkin marked this pull request as ready for review May 20, 2024 20:46
@igormunkin igormunkin requested a review from a team as a code owner May 20, 2024 20:46
@igormunkin igormunkin merged commit 148c858 into ydb-platform:main May 20, 2024
7 checks passed
@igormunkin igormunkin deleted the YQL-18435-fix-pyerr-rethrow-misuse branch May 22, 2024 05:07
igormunkin added a commit to igormunkin/ydb that referenced this pull request May 24, 2024
Considering the difference between Python 2.x and Python 3.x regarding
the long integral types behaviour change, the patch adjusts the test
function payload to trigger the same exception for both major versions.

Follows up ydb-platform#4699
MrLolthe1st pushed a commit to MrLolthe1st/ydb that referenced this pull request May 28, 2024
@niksaveliev niksaveliev mentioned this pull request May 29, 2024
@niksaveliev niksaveliev mentioned this pull request Jun 7, 2024
This was referenced Jun 10, 2024
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.

2 participants