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

Add partiql-types and literals typing #389

Merged
merged 4 commits into from
Jun 14, 2023
Merged

Add partiql-types and literals typing #389

merged 4 commits into from
Jun 14, 2023

Conversation

am357
Copy link
Contributor

@am357 am357 commented Jun 9, 2023

Description of changes:
This is the first PR to introduce types to partiql-lang-rust. With the changes in this PR, we're introducing partiql-types which includes the initial model for our built-in types. We also introduce an AstStaticTyper in partiql-ast-passes which provides an API for receiving an ast::AstNode and outputting a mapping from AST Node Ids to their corresponding types.

In the first version, we're loosely typing the literals with the expectation that this goes through multiple iterations.
We're moving the custom type support out of the scope at this stage.

Here is the high-level plan:

  • Type literals (initially in this PR)
  • Type variable references in SFW
  • Type operators
  • Type using schemas in typing environment
  • Add type coercions (using a Type Lattice?)
  • Update CHANGELOG.md and adh documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@am357 am357 force-pushed the feat-types branch 3 times, most recently from 6c0c65e to 4dd0c77 Compare June 9, 2023 22:40
@github-actions
Copy link

github-actions bot commented Jun 9, 2023

Conformance comparison report

Base (67777c2) 1fef4c5 +/-
% Passing 80.44% 83.39% 2.95%
✅ Passing 2833 5240 2407
❌ Failing 495 1044 549
🔶 Ignored 194 0 -194
Total Tests 3522 6284 2762

Number passing in both: 379

Number failing in both: 13

Number passing in Base (67777c2) but now fail: 0

Number failing in Base (67777c2) but now pass: 0

@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Patch coverage: 59.04% and project coverage change: -0.41 ⚠️

Comparison is base (67777c2) 81.97% compared to head (47736de) 81.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #389      +/-   ##
==========================================
- Coverage   81.97%   81.57%   -0.41%     
==========================================
  Files          55       56       +1     
  Lines       14370    14640     +270     
  Branches    14370    14640     +270     
==========================================
+ Hits        11780    11942     +162     
- Misses       2108     2198      +90     
- Partials      482      500      +18     
Impacted Files Coverage Δ
partiql-ast-passes/src/lib.rs 100.00% <ø> (ø)
partiql-ast/src/ast.rs 1.07% <ø> (ø)
partiql-types/src/lib.rs 55.55% <54.43%> (-44.45%) ⬇️
partiql-ast-passes/src/static_typer.rs 60.93% <60.93%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@am357 am357 force-pushed the feat-types branch 9 times, most recently from 148a11b to 698b8bc Compare June 12, 2023 19:18
This is the first commit to introduce types to `partiql-lang-rust`.
With the changes in this commit, we're introducing `partiql-types`
create which includes the initial model for our built-in types.
We also introduce an `AstTyper` in `partiql-ast-passes` which
provides an API for receiving `partiql-ast` and outputting a
mapping from AST Node Ids to their corresponding types.

In the first version, we're loosely typing the literal with the
expection that this goes through multiple iterations.

Here is the high-level plan:
[ ] Type literals
[ ] Type variable references in SFW
[ ] Type operators
[ ] Type using schemas in typing environment
[ ] Add type coercions (using a Type Lattice?)
[ ] Update CHANGELOG.md and adh documentation
@am357 am357 changed the title [WIP] Add partiql-types and literals typing Add partiql-types and literals typing Jun 12, 2023
@am357 am357 marked this pull request as ready for review June 12, 2023 19:21
@am357 am357 requested a review from jpschorr June 12, 2023 19:21
@am357 am357 merged commit babd7ac into main Jun 14, 2023
@am357 am357 deleted the feat-types branch June 14, 2023 15:21
am357 added a commit that referenced this pull request Jun 14, 2023
Addressing the comment in PR #389, this PR re-uses the `AstTypeMap`
for `LocationMap` which removes a dependency to `HashMap` as `AstTypeMap`
uses `IndexMap`.
am357 added a commit that referenced this pull request Jun 14, 2023
Addressing the comment in PR #389, this PR re-uses the `AstTypeMap`
for `LocationMap` which removes a dependency to `HashMap` as `AstTypeMap`
uses `IndexMap`.
am357 added a commit that referenced this pull request Aug 16, 2023
During the work for adding a steel thread for PartiQL Typing (Recent PR #389)
and after merging #410 to `feat-type-plan-poc` it is realized that we need to
refactor the code to remove `DynamicLocalup` `VarExpr` with the assumption that
we work based off of Typing and Value Environment from the Catalog. We have a
Typing Environment in the Catalog at the moment and we are going to add the
Variable Environment as well. In preparation for such task, we need to make
the `NameResolver` Catalog aware. In that regard this commit adds the `Catalog`
to `NameResolver`

Expecting subsequent PR(s) for the name resolving using the Catalog.
am357 added a commit that referenced this pull request Aug 16, 2023
During the work for adding a steel thread for PartiQL Typing (Recent PR #389)
and after merging #410 to `feat-type-plan-poc` it is realized that we need to
refactor the code to remove `DynamicLocalup` `VarExpr` with the assumption that
we work based off of Typing and Value Environment from the Catalog. We have a
Typing Environment in the Catalog at the moment and we are going to add the
Variable Environment as well. In preparation for such task, we need to make
the `NameResolver` Catalog aware. In that regard this commit adds the `Catalog`
to `NameResolver`

Expecting subsequent PR(s) for the name resolving using the Catalog.
am357 added a commit that referenced this pull request Aug 16, 2023
During the work for adding a steel thread for PartiQL Typing (Recent PR #389)
and after merging #410 to `feat-type-plan-poc` it is realized that we need to
refactor the code to remove `DynamicLocalup` `VarExpr` with the assumption that
we work based off of Typing and Value Environment from the Catalog. We have a
Typing Environment in the Catalog at the moment and we are going to add the
Variable Environment as well. In preparation for such task, we need to make
the `NameResolver` Catalog aware. In that regard this commit adds the `Catalog`
to `NameResolver`

Expecting subsequent PR(s) for the name resolving using the Catalog.
am357 added a commit that referenced this pull request Aug 16, 2023
During the work for adding a steel thread for PartiQL Typing (Recent PR #389)
and after merging #410 to `feat-type-plan-poc` it is realized that we need to
refactor the code to remove `DynamicLocalup` `VarExpr` with the assumption that
we work based off of Typing and Value Environment from the Catalog. We have a
Typing Environment in the Catalog at the moment and we are going to add the
Variable Environment as well. In preparation for such task, we need to make
the `NameResolver` Catalog aware. In that regard this commit adds the `Catalog`
to `NameResolver`

Expecting subsequent PR(s) for the name resolving using the Catalog.
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