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

HIVE-28215: Signalling CONDITION HANDLER is not working in HPLSQL. #5231

Merged

Conversation

mdayakar
Copy link
Contributor

@mdayakar mdayakar commented May 2, 2024

HIVE-28215: Signalling CONDITION HANDLER is not working in HPLSQL.

What changes were proposed in this pull request?

Currently the CONDITION HANDLERs defined by user are not getting invoked when the signal is given to the corresponding condition. As a part of processing exception/error handling conditions, user defined conditions also processed (just looging into the log file) so when it has to invoke user defined condition handlers there are no condition handlers in the stack.

Why are the changes needed?

To fix the issue of invoking user defined condition handlers properly.

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

mvn test -Dtest=TestHplSqlViaBeeLine -pl itests/hive-unit -Pitests

Comment on lines 1052 to 1053
if (userDefinedSignals.size() > 0) {
for (int i = userDefinedSignals.size() - 1; i >= 0; i--) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check necessary? If userDefinedSignals is empty the loop doesn't executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -1028,9 +1028,13 @@ void cleanup() {
* Output information about unhandled exceptions
*/
public void printExceptions() {
List<Signal> userDefinedSignals = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it allowed to add the same Signal more than one times?
Does the order of Signals matter?

Copy link
Contributor Author

@mdayakar mdayakar May 6, 2024

Choose a reason for hiding this comment

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

Is it allowed to add the same Signal more than one times?

As per code, same signal is not getting added more than once.

Does the order of Signals matter?

Here to store the signals Stack data structure is getting used so might be the order matters thats why pushing the userdefined signals in the same order which they were present in Stack before processing other signals.

Copy link

sonarcloud bot commented May 6, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@kasakrisz kasakrisz merged commit 1b0e9d9 into apache:master May 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants