trinodb / trino-python-client

Python client for Trino
Apache License 2.0
309 stars 151 forks source link

`'0E-10' is not a valid decimal literal` #347

Closed maskarb closed 1 year ago

maskarb commented 1 year ago

Expected behavior

In Python, Decimals can be represented with e or E (e.g. Decimal('0E-10')). When the Trino client receives a Decimal like this one, it should successfully convert the value to something Trino can process.

Actual behavior

Given a basic sql query (SELECT ?) with the param ([Decimal('0E-10')]), the Trino client prepared and executed the following:

preparedStatements: {st_24b0c92d9feb474cbcbac06e30057885: "SELECT ?"}

query: "EXECUTE st_24b0c92d9feb474cbcbac06e30057885 USING DECIMAL '0E-10'"

which results in the following error:

failureInfo: {
  type: "io.trino.spi.TrinoException",
  message: "line 1:51: '0E-10' is not a valid decimal literal"
}

Steps To Reproduce

>>> import trino
>>> from decimal import Decimal

>>> sql = "select ?"
>>> params = [Decimal('0E-10')]

>>> connection_args = {'host': 'trino', 'port': 8080, 'user': 'admin', 'catalog': 'hive', 'schema': 'org1234567'}
>>> conn = connect(**connection_args)
>>> cur = conn.cursor()
>>> cur.execute(sql, params)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/koku/.venv/lib/python3.9/site-packages/trino/dbapi.py", line 473, in execute
    self._iterator = iter(self._query.execute())
  File "/opt/koku/.venv/lib/python3.9/site-packages/trino/client.py", line 805, in execute
    self._result.rows += self.fetch()
  File "/opt/koku/.venv/lib/python3.9/site-packages/trino/client.py", line 820, in fetch
    status = self._request.process(response)
  File "/opt/koku/.venv/lib/python3.9/site-packages/trino/client.py", line 621, in process
    raise self._process_error(response["error"], response.get("id"))
trino.exceptions.TrinoUserError: TrinoUserError(type=USER_ERROR, name=INVALID_LITERAL, message="line 1:51: '0E-10' is not a valid decimal literal", query_id=20230315_192843_00008_6fa45)

Log output

I've attached the .json output from the Trino UI.

query.json.txt

Operating System

RHEL

Trino Python client version

0.321.0

Trino Server version

405

Python version

Python 3.9.13

Are you willing to submit PR?

ghost commented 1 year ago

The issue seems to be with the Trino server not recognizing the decimal literal '0E-10' as a valid decimal. One possible solution is to represent the decimal as a string instead of using scientific notation. You can try changing the query parameter to the string '0.0', which should be equivalent to the decimal 0E-10. Another solution could be to update the Trino server to recognize scientific notation for decimals, if possible. If you are willing to submit a pull request, you can also try to update the Trino Python client to handle scientific notation for decimals.

maskarb commented 1 year ago

In the Trino Python client, we're doing the Decimal conversion here: https://github.com/trinodb/trino-python-client/blob/master/trino/dbapi.py#L457-L458

        if isinstance(param, Decimal):
            return "DECIMAL '%s'" % param

I was thinking of just converting the Decimal to a float and using the same logic for floats which is here: https://github.com/trinodb/trino-python-client/blob/master/trino/dbapi.py#L404-L411

        if isinstance(param, float):
            if param == float("+inf"):
                return "infinity()"
            if param == float("-inf"):
                return "-infinity()"
            if math.isnan(param):
                return "nan()"
            return "DOUBLE '%s'" % param

My only issue with doing that is I don't know the difference between DOUBLE and DECIMAL. We could just copy the logic and do this:

        if isinstance(param, Decimal):
            param = float(param)
            if param == float("+inf"):
                return "infinity()"
            if param == float("-inf"):
                return "-infinity()"
            if math.isnan(param):
                return "nan()"
            return "DECIMAL '%s'" % param

But I'm just not sure.

hashhar commented 1 year ago

Trino implicitly converts literals with scientific notation to DOUBLE. DECIMAL literals cannot use scientific notation.

Maybe we can detect this on the Python client side and normalize the literal to use decimal instead of scientific notation.