ibis icon indicating copy to clipboard operation
ibis copied to clipboard

feat(duckdb): show action,SQL on ParserException/BinderException

Open NickCrews opened this issue 10 months ago • 2 comments

This would make errors such as https://github.com/ibis-project/ibis/issues/8771 easier to debug

This could also be considered a security risk, since someone might put their eg API keys in their SQL statement, and then this shows up in some public log. IDK how much we care about this.

NickCrews avatar Mar 25 '24 21:03 NickCrews

I don't think we should do this. There's certainly security and privacy concerns here, there's also the fact that printing out SQL generated by Ibis might make the traceback unreadably long.

cpcloud avatar Apr 02 '24 15:04 cpcloud

OK, that's fair. Could we do something else to make debugging easier though?

  • include the SQL in the custom error, eg
raise IbisError(
                "Failed to parse SQL. This is a bug in Ibis. Please report it at "
                "https://github.com/ibis-project/ibis/issues.",
                 sql=sql
            ) from e

and this custom error type doesn't include the SQL in the repr

  • Create an ibis logger and log this at the warning level
  • add some debug mode where this is logged? (could be other uses for this)

NickCrews avatar Apr 03 '24 01:04 NickCrews

Closing this out for now. This PR isn't viable as is. I think bug reports coming from errors from invalid SQL produced by Ibis are fine to be reported from an exception generated by DuckDB for now.

cpcloud avatar May 27 '24 13:05 cpcloud