Skip to content

fix(postgresql): fix TEIID error when getting postgresql cancellation token#18207

Closed
davemulford wants to merge 1 commit intoapache:masterfrom
davemulford:fix-teiid-cancelation
Closed

fix(postgresql): fix TEIID error when getting postgresql cancellation token#18207
davemulford wants to merge 1 commit intoapache:masterfrom
davemulford:fix-teiid-cancelation

Conversation

@davemulford
Copy link

@davemulford davemulford commented Jan 27, 2022

Using TEIID virtual databases (VDBs) in Superset requires the use of the postgres connector. In a previous update, cancelation tokens were added to allow a query to be stopped on the database server. This is a good thing, however it introduced a bug that disallowed the use of TEIID VDBs.

This change wraps the SELECT pg_backend_pid() query in a try-catch phrase, and returns None upon exception.

The error encountered in the Superset SQL Editor is:

superset.exceptions.SupersetErrorsException: [SupersetError(message="TEIID30068 The function 'pg_backend_pid()' is an unknown form.  Check that the function name and number of arguments is correct.
DETAIL:  org.teiid.jdbc.TeiidSQLException: TEIID30068 The function 'pg_backend_pid()' is an unknown form.  Check that the function name and number of arguments is correct.",
error_type=<SupersetErrorType.GENERIC_DB_ENGINE_ERROR: 'GENERIC_DB_ENGINE_ERROR'>, level=<ErrorLevel.ERROR: 'error'>, extra={'engine_name': 'PostgreSQL', 'issue_codes': [{'code': 1002, 'message': 'Issue 1002 - The database returned an unexpected error.'}]})]

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Using TEIID virtual databases (VDBs) in Superset requires the use of the postgres connector. In a previous update, cancelation tokens were added to allow a query to be stopped on the database server. This is a good thing, however it introduced a bug that disallowed the use of TEIID VDBs.

This change wraps the `SELECT pg_backend_pid()` query in a try-catch phrase, and returns `None` upon exception.
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

cursor.execute("SELECT pg_backend_pid()")
row = cursor.fetchone()
return row[0]
except Exception:
Copy link
Member

Choose a reason for hiding this comment

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

Minor improvement proposal: is there a more specific exception type that we could be catching here?

Copy link
Author

Choose a reason for hiding this comment

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

I can double-check and make the change. Thanks for the review!

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the PR comment with the error being raised. It's a superset.exceptions.SupersetErrorsException. Would you like me to catch that instead of the generic exception?

Copy link
Member

Choose a reason for hiding this comment

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

I've updated the PR comment with the error being raised. It's a superset.exceptions.SupersetErrorsException. Would you like me to catch that instead of the generic exception?

Really? I would have thought it's a psycopg2 or at least a SQLAlchemy exception, as I assume it's coming from the cursor call. But yeah, let's make it as specific as possible if it's not too much trouble.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 16, 2022
@stale stale bot closed this Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inactive Inactive for >= 30 days size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants