feat(parse)!: add proto::extensions::SimpleExtensionUri parser
#169
Conversation
| Entry::Vacant(entry) => { | ||
| // This is where we would resolve and then parse. | ||
| // This check shows the use of the unsupported uri error. | ||
| if let "http" | "https" | "file" = simple_extension_uri.uri().scheme() { |
There was a problem hiding this comment.
Some dialects do not require or expect a scheme (I'll get examples when I return from vacation). It may be useful to make this configurable.
There was a problem hiding this comment.
That should be documented here: https://github.com/substrait-io/substrait/blob/e564cf0b3b769491c19016d92d195b5f62361775/proto/substrait/extensions/extensions.proto#L18-L20
Please note that this is just a test context, and not the actual context that I want to eventually provide in this crate. In that case yes, the resolver should be configurable.
Enjoy your vacation!
|
|
||
| /// A parsed [proto::extensions::SimpleExtensionUri]. | ||
| #[derive(Clone, Debug, PartialEq)] | ||
| pub struct SimpleExtensionUri { |
There was a problem hiding this comment.
A builder is going to need to do registration (not that you need that now).
There was a problem hiding this comment.
What do you mean with registration? The approach taken here is to have the parse context resolve, fetch and parse the referred extension (when parsing the definition):
substrait-rs/src/parse/proto/extensions/simple_extension_uri.rs
Lines 70 to 72 in dc945cd
There was a problem hiding this comment.
If an instance of this is created through the course of creating a plan (not the parse case you're concentrating on at the moment) the registration of anchors/reference ids would be handy. It may happen in another class that generates these (perhaps providing function lookup and SQL to Substrait lookup). I will be fleshing out a builder in substrait-python soon.
A builder (and registration) isn't a concern for this PR.
Adds a parser for
proto::extensions::SimpleExtensionUrithat parses the uri and adds the extension to the parse context.Breaking change because the parse
Contexttrait gets two new functions to support this:add_simple_extension_uri: this is used to add a simple extensions, the parse context must directly resolve the uri and return the parsed simple extensions - struct stub for that is added in this PR (the parser is TODO).simple_extensions: given an reference (anchor) to a simple extensions - check if this was added to the parse context and return it