feat: hydrate python exceptions

[Mause]

This is part of my efforts to improve the exceptions being thrown from the Python API - I still have to replace all the locations where std::runtime_error is being thrown directly.

I'm not sure if the path I've taken with the C++ changes is the right one, so feedback is definitely welcome

[pdet]

@Mytherin it seems to me that this issue will affect all our APIs, is there a specific reason to turn query errors into strings? If not we should probably change this to pass on a DuckDBException of sorts, no?

Instead of reconstructing exceptions at the API level...

[Mause]

@pdet it probably affects them all yes though I can't answer as to why we're turning them into strings in the first place

and we already have such an exception - duckdb::Exception

[Mytherin]

Perhaps we should instead add the exception type to the query result, to avoid having to do string parsing to reconstruct the exception type?

[pdet]

What is the rationale for just having the string (with the addition of an exception type) and not the duckdb::Exception in there?

[pdet]

@Mause I've chatted with Mark about this, the reason to not use Duckdb exceptions is mainly due to their copying being unnecessarily complicated.

In this case, maybe we should create a struct of QueryResultException in the query result which propagates the Exception Type, and then we avoid the exception reconstruction.

[Mause]

@Mause I've chatted with Mark about this, the reason to not use Duckdb exceptions is mainly due to their copying being unnecessarily complicated.

In this case, maybe we should create a struct of QueryResultException in the query result which propagates the Exception Type, and then we avoid the exception reconstruction.

That seems reasonable, though we do still have to reconstruct the exceptions in that case, though we can remove the string splitting

[Tishj]

Thanks for the PR!

First of all I really like the changes to the tests! However the hydrated exceptions make a couple of assumptions that might not hold true for all error messages (as not all error messages are created from a caught Exception)

The current situation is definitely an issue, and I appreciate you bringing attention to it, but this is part of a wider issue in the codebase that affects more than just the Python client.

I've raised a PR(#4371) that addresses this issue at the root, rather than at the tail-end of it.

[Mause]

@Tishj what's your plan with regards to all the tests? Are you planning to merge without updating them all as I have done here?

[Tishj]

@Tishj what's your plan with regards to all the tests? Are you planning to merge without updating them all as I have done here?

I was thinking of taking the best of both, merging your test changes into my PR, if that's okay with you

[Mause]

@Tishj what's your plan with regards to all the tests? Are you planning to merge without updating them all as I have done here?

I was thinking of taking the best of both, merging your test changes into my PR, if that's okay with you

If they all still pass, feel free to cherry pick them across

[Mause]

Closed in favour of changes copied to https://github.com/duckdb/duckdb/pull/4371