Skip to content

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

Open
mdayakar wants to merge 1 commit intoapache:masterfrom
mdayakar:HIVE-29555_HPLSQLIssue
Open

HIVE-29555: [HPLSQL]Errorcode handling is not proper in HPLSQL.#6422
mdayakar wants to merge 1 commit intoapache:masterfrom
mdayakar:HIVE-29555_HPLSQLIssue

Conversation

@mdayakar
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@thomasrebele thomasrebele left a comment

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants