crate / cratedb-sqlparse

Parsing utilities to validate and split SQL statements for CrateDB.
Apache License 2.0
4 stars 1 forks source link

Change error handling strategy and add verbose errors #30

Closed surister closed 1 month ago

surister commented 1 month ago

Summary of the changes / Why this is an improvement

Issue #2

This is one of the first pieces required to finish the feature and its only implemented in Python for early feedback/validation.

Main changes of PR:

The main issue I would like help with the review: <-------------------------------------


we lose the information on which statement the error belongs to, I tried with different ErrorStrategies and Error listeners but to no avail, also didn't find anything online.

My best effort is to try to match the error's offendingToken query, to any of the parsed statements:

```python
# Shortened for clarity purposes.
def find_suitable_error(statement, errors):
    for error in errors[:]:
        if error.query == statement.query:
            statement.exception = error
            errors.pop(errors.index(error))

There are a couple of edge cases that I covered but in short, it works reasonable well, even though I'm not a fan of it, maybe you folks way more experience in antlr4 have an idea?

Checklist

surister commented 1 month ago

cc @proddata I'd love your feedback as well as any input on this

proddata commented 1 month ago

I read through it and generally like the idea of not raising an exception. However, I'm not sure about the defaults.

Regarding the "main issue," I don't have a strong opinion as I lack details on the mechanics. If the approach works, though, 👍.