Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow module in ident #4324

Merged
merged 2 commits into from
Mar 9, 2024
Merged

feat: allow module in ident #4324

merged 2 commits into from
Mar 9, 2024

Conversation

aljazerzen
Copy link
Member

No description provided.

@aljazerzen aljazerzen enabled auto-merge (squash) March 8, 2024 18:59
@aljazerzen aljazerzen merged commit cbd3cd9 into main Mar 9, 2024
34 checks passed
@aljazerzen aljazerzen deleted the feat-super-project branch March 9, 2024 08:56
@snth
Copy link
Member

snth commented Mar 9, 2024

Interesting. First thought I had when reading through the example is one could possibly also use "this" for this purpose. Not arguing for it at this point, just putting it out there as an idea to explore.

"module" is possibly more explicit and using "this" would be overloading the use of "this".

Counterargument to that is that this is overloading the use of "module". Furthermore in a pipeline "this" references the current namespace and makes the component objects (columns) available with "this.column_name". The usage here would be analogous with "this" referencing the current (module) namespace and making its objects available through "this.object".

WDYT?

@aljazerzen
Copy link
Member Author

The added test is just a parser test, so nothing is implemented yet.

You are right that module is very similar to this, but this refers to the "current" relation in the pipeline.


Background for this change:

I'm experimenting with a rewrite of name resolution and my new strategy is to not to implement it all at once. Instead, I'll simplify the language (and make it much more inconvenient to use), so I get basics working. Then I can try to simplify it back into what we have now, or something very similar. I don't plan on merging the "inconvenient" version of the language into main, because that would require a lot of campaigning to get the language design accepted by all.

This PR is needed because I need a way of distinguishing between column references and declaration references.

First I tried to do that with #1619 (i.e. .my_column for columns), but such changes require rewriting all of our tests manually, which would mean adding dots in front of all column names we have in sql.rs, which is a lot.

My next idea is to instead require explicit module or project or super for all declaration references. This is similar to what we just did with adding db. prefix to all table references. It would look something like this:

let x = (from project.db.my_table | select {a, b})

from module.x
select {a, b}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants