Skip to content

Conversation

@wk8
Copy link

@wk8 wk8 commented Oct 5, 2024

Sometimes it's nice to be able to access the parsed query, say in a Table's callbacks; for example, if one implements a table backed by a system that allows fast access for very specific queries, it might not be possible (or at least not easy) to take advantage of that using one of the pre-defined *Table interfaces. In that case, the most straightforward way to implement this would be to return the relevant subset of rows from PartitionRows, depending on the query.

This patch allows to access the already parsed query, instead of parsing it all over again.

Happy to add tests if you're okay with the general idea.

Sometimes it's nice to be able to access the parsed query, say in a `Table`'s callbacks; for example, if one implements a table backed by a system that allows fast access for very specific queries, it might not be possible (or at least not easy) to take advantage of that using one of the pre-defined `*Table` interfaces. In that case, the most straightforward way to implement this would be to return the relevant subset of rows from `PartitionRows`, depending on the query.

This patch allows to access the already parsed query, instead of parsing it all over again.
@max-hoffman
Copy link
Contributor

This is pretty safe to merge, I do think the way you plan to use the AST might give you some issues. Normally we would fold execution time decision making into the plan layer with transformation rules. Depending on how you are using GMS, if might be possible to extend the engine's analyzer with custom rules. That would look similar to your AST walk, but on a plan sql.Node instead (we have a lot of search and pattern matching helper functions). Setting a property on your custom table at planning time generally lends itself to easier extensibility, testing, debugging, etc.

@max-hoffman
Copy link
Contributor

Sometimes it's nice to be able to access the parsed query, say in a Table's callbacks; for example, if one implements a table backed by a system that allows fast access for very specific queries, it might not be possible (or at least not easy) to take advantage of that using one of the pre-defined *Table interfaces. In that case, the most straightforward way to implement this would be to return the relevant subset of rows from PartitionRows, depending on the query.

This patch allows to access the already parsed query, instead of parsing it all over again.

Happy to add tests if you're okay with the general idea.

Following up on this, we'd like more information on what you are trying to do. The changes here are harmless but I suspect you'd be better off using one of the well-defined ways of what sounds like custom filtering table data? For example, there is an interface where a table can peek at its related filter expressions and handoff responsibility to the table object.

@max-hoffman
Copy link
Contributor

@wk8 Closing this issue for now pending more details. We are happy to reopen, just let us know more about what you are trying to do.

@max-hoffman max-hoffman closed this Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants