-
Notifications
You must be signed in to change notification settings - Fork 61
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
Structures globals within plan to lie within catalogs #1308
Conversation
Conformance comparison report
Number passing in both: 5384 Number failing in both: 434 Number passing in Base (0a75bd3) but now fail: 0 Number failing in Base (0a75bd3) but now pass: 0 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1308 +/- ##
=========================================
Coverage 72.16% 72.16%
Complexity 2097 2097
=========================================
Files 221 221
Lines 15974 15974
Branches 2892 2892
=========================================
Hits 11528 11528
Misses 3640 3640
Partials 806 806
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
catalog::{ | ||
name: string, | ||
values: list::[value], | ||
_: [ | ||
value::{ | ||
path: list::[string], // This should NOT be empty. | ||
type: static_type | ||
} | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catalog::{ | |
name: string, | |
values: list::[value], | |
_: [ | |
value::{ | |
path: list::[string], // This should NOT be empty. | |
type: static_type | |
} | |
] | |
catalog::{ | |
name: string, | |
symbols: list::[symbol], | |
} | |
symbol::[ | |
value::{ | |
path: list::[string], // This should NOT be empty. | |
type: static_type | |
} | |
// TODO fn | |
_::[ | |
ref::{ | |
catalog: int, | |
symbol: int, | |
} | |
] | |
] |
This structure also aligns with https://en.wikipedia.org/wiki/Symbol_table
The minimum information contained in a symbol table used by a translator and intermediate representation (IR) includes the symbol's name and its location or address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the use of ref
a lot. Modified slightly for the nesting, but just pushed.
CHANGELOG.md
Outdated
@@ -48,6 +48,7 @@ Thank you to all who have contributed! | |||
- The new plan is fully resolved and typed. | |||
- Operators will be converted to function call. | |||
- Changes the return type of `filter_distinct` to a list if input collection is list | |||
- **BREAKING**: Globals within the logical plan are now represented in a structured manner to reflect the existence of catalogs. Each value is now mandated to be case-sensitive (we have moved from Identifiers to string literals). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were unreleased AFAIK
Relevant Issues
Description
Other Information
Updated Unreleased Section in CHANGELOG: YES
Any backward-incompatible changes? YES
Any new external dependencies? NO
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? YES
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.