eliaperantoni commented on code in PR #13664:
URL: https://github.com/apache/datafusion/pull/13664#discussion_r1873112786
##########
datafusion/common/src/with_span.rs:
##########
@@ -0,0 +1,96 @@
+use std::{cmp::Ordering, fmt::{self, Debug, Display}, ops::{Deref, DerefMut}};
+
+use sqlparser::tokenizer::Span;
+
+/// A wrapper type for entitites that somehow refer to a location in the source
Review Comment:
> Did you consider a trait like was done in sqlparser that retrieves spans?
Yes, but to implement one such trait I'd have to put the data in `DataType`.
And I think that's a bit pushing the limits of what `DataType` is meant to do.
I quite like the idea of the `WithSpan` (can rename) functioning like a
sidecar, to limit big changes in the code.
Maybe we could have a `Spanned` trait in datafusion as well, that is
automatically implemented for all `WithSpan<T>`, and also by types that
natively carry a `Span`?
> Though maybe now would be a good time to make a proper structure for
coercing types, (struct TypeCoercer or something) where we could attach the
spans 🤔
Not too familiar about the history of that component. But I think that's
just one example, and there's probably many more cases where changes like that
would be needed to accommodate the presence of `Span`s.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
