HIVE-29555: [HPLSQL]Errorcode handling is not proper in HPLSQL.

[mdayakar]

HIVE-29555: [HPLSQL]Errorcode handling is not proper in HPLSQL.

What changes were proposed in this pull request?

When SQL statements are executed using EXECUTE/EXECUTE IMMEDIATE, in positive scenario the ERRORCODE is not set as SUCCESS. The default value for ERRORCODE is SUCCESS so even without setting the errorcode to success in positive scenario is fine but if the ERRORCODE is set as ERROR as a part of previous statement execution then another statement is executed successfully but still ERRORCODE is already set as ERROR which is not correct. Now in positive flow also we are setting SUCCESS for the ERRORCODE.

Why are the changes needed?

To fix the error reported in HIVE-29555.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added testcase to test the scenario.
mvn test -Dtest=TestHplSqlViaBeeLine#testERRORCODEForExecuteStatements -pl itests/hive-unit -Pitests

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[thomasrebele]

I've checked some other methods in the classes Stmt and Exec for potential bugs

• Stmt#fetch when entering the if (ctx.bulk_collect_clause() != null), no status is set
• Stmt#return_ calls Exec#signal(Signal.Type), which always setSqlCode(SqlCodes.ERROR)
• Stmt#allocateCursor does not set the status
• There are a few setSqlCode(SqlCodes.NO_DATA_FOUND), even though there's a dedicated method Exec#setSqlNoData; moreover, 2 out of 3 of these calls are directly followed by a exec.signal(Signal.Type.NOTFOUND), which overrides the SQL code to SqlCodes.ERROR

Some places seem to not set the SQL state:

• Stmt#use uses exec.setSqlCode(SqlCodes.SUCCESS) instead of exec.setSqlSuccess()
• Expression#execCursorAttribute, if (cursorVar != null)
• Exec#execHost

I think these cases should be handled as well, either in this ticket, or by a follow-up ticket.

[kasakrisz]

Is it necessary to actually load the data?

How about testing with statements like

select 1;

and

select 1/0;