Errors with source code location

[eliaperantoni]

Idea

We propose implementing the ability of enriching errors, such as those coming from the parsing of a SQL query into AST nodes or the build of its logical plan, with the location of the code that caused them. i.e. a Span.

Example

For example, in the following query:

WITH products_with_price AS (
	SELECT
		id,
		name,
		price_usd -- Also highlight here
	FROM
		products
	JOIN
		prices
	ON
		products.id = prices.product_id
)
SELECT
	id,
	name,
	price_usd + '$' AS display_price_usd -- Error on this line
FROM
	products_with_price

The error is that an integer and a string cannot be coerced to types such that an arithmetic operation is applicable to them, (without explicit casts). In this case, the error should highlight the third-to-last line of the query.

At the same time, it would be helpful to highlight the projected price_usd column in the CTE too. Not as the main error but more as a note to help the user understand the context around the error and the elements involved.

Proposal

We propose adding a new variant to DataFusionError, let's call it DataFusionError::Diagnostics. This variant would wrap any other DataFusionError, plus a vec of DiagnosticsEntry. Each of those is a string message together with a Span and a kind.

enum DataFusionError {
	// ...
	Diagnostics(Vec<DiagnosticsEntry>, Box<DataFusionError>)
}

struct DiagnosticsEntry {
	message: String,
	span: Span,
	kind: DiagnosticsEntryKind,
}

enum DiagnosticsEntryKind {
	Error,
	Warning,
	Note,
	Help,
}

Then, we could progressively add points in the logical planner where the errors are enriched with source code information by wrapping them in DataFusionError::Diagnostics before returning.

For that to be possible, we'd have to move the Span to the logical Expr, when converting from the parser's Expr. See here for example. Without this, the information about the source code location is lost when the conversion happens, and the logical planner has no way to get it.

Purpose of document

We'd love to hear everybody's thoughts on this hypothetical feature and the proposed solution. Whether it would cover most foreseeable solutions, whether there is a better alternative, if it should be extended to carry even more information, etc.

[eliaperantoni]

Moved to issue #13662, so it can be linked to a PR.