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

Improve error messages #126

Merged

Conversation

jgoizueta
Copy link

When an error occurs in a ODBC call, we were only producing generic error messages about what we we're trying to do.
With this patch we now also include information about the error provided by the ODBC driver.

This is to support https://app.clubhouse.io/cartoteam/story/80914/improve-error-messages-while-importing

For example, if a SQL query executed (via sql_quey in IMPORT FOREIGN SCHEMA) was syntactically incorrect, we had:

ERROR:  Executing ODBC query

With the extension compiled with debug information and debug messages on we had some additional information, but not as part of the error message:

DEBUG:  Error result (-1): Executing ODBC query
DEBUG:   42000:1:1003:SQL compilation error:
syntax error line 1 at position 51 unexpected '10'.

ERROR:  Executing ODBC query

Now we have the same information in the DEBUG log appendend to the error message:

ERROR:  Executing ODBC query to get schema
SQL compilation error:
syntax error line 1 at position 51 unexpected '10'.

This will allow for better error messages in the CARTO Import API: see https://app.clubhouse.io/cartoteam/story/80914/improve-error-messages-while-importing

@jgoizueta jgoizueta requested a review from rafatower October 5, 2020 16:42
@jgoizueta
Copy link
Author

Tests need to be adapted now for the augmented error messages

@jgoizueta jgoizueta removed the request for review from rafatower October 5, 2020 16:59
@jgoizueta
Copy link
Author

@rafatower sorry I requested your review too early, to avoid bothering you too much I'll ping you when tests are ready

@jgoizueta
Copy link
Author

😅 I thought adapting the test was going to be much harder

@rafatower rafatower self-requested a review October 6, 2020 11:43
Copy link

@rafatower rafatower left a comment

Choose a reason for hiding this comment

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

👍 🎉

@jgoizueta jgoizueta merged commit b6e19e8 into master Oct 14, 2020
@jgoizueta jgoizueta deleted the feature/ch80914/improve-error-messages-while-importing branch October 14, 2020 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants