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

Bump up sqlparser crate version from 0.11 to 0.13 #479

Merged
merged 4 commits into from
Feb 5, 2022

Conversation

MRGRAVITY817
Copy link
Collaborator

Bump up sqlparser version.

Changes made

Most of the changes are in SqlStatement::Update part,

  • table field is now TableWithJoin type, so I had to take relation field out of it and converted into ObjectName.
  • SqlAssignment's id is now Vec<Ident> type, so I took only the first element's value.

TODO's after implementing Join

  • Remove JoinOnUpdateNotSupported error.

@panarch panarch requested review from ever0de and panarch February 3, 2022 11:25
@ever0de ever0de removed the request for review from panarch February 3, 2022 11:25
@panarch panarch added the improvement Improvements for existing features label Feb 3, 2022
return Err(TranslateError::JoinOnUpdateNotSupported.into());
}
match &table.relation {
TableFactor::Table { name, .. } => Ok(ObjectName(translate_idents(&name.0))),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use translate_object_name here

}
match &table.relation {
TableFactor::Table { name, .. } => Ok(ObjectName(translate_idents(&name.0))),
_ => Err(TranslateError::JoinOnUpdateNotSupported.into()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks that this could be not directly related to JOIN.
Let's add a new error with UnsupportedTableFactor(String) and have a corresponding test case for this.

@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2022

Codecov Report

Merging #479 (3056085) into main (3c68d0d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
+ Coverage   91.59%   91.61%   +0.02%     
==========================================
  Files         160      160              
  Lines        9366     9379      +13     
==========================================
+ Hits         8579     8593      +14     
+ Misses        787      786       -1     
Impacted Files Coverage Δ
core/src/translate/mod.rs 100.00% <100.00%> (+1.61%) ⬆️
test-suite/src/error.rs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c68d0d...3056085. Read the comment docs.

Copy link
Member

@panarch panarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good, thanks a lot! Merging 👍

@panarch panarch merged commit a04400e into gluesql:main Feb 5, 2022
ever0de pushed a commit to ever0de/gluesql that referenced this pull request Feb 20, 2022
Most of the changes are in SqlStatement::Update part,
- table field is now TableWithJoin type, so I had to take relation field out of it and converted into ObjectName.
- SqlAssignment's id is now Vec<Ident> type, so I took only the first element's value.
ever0de pushed a commit to ever0de/gluesql that referenced this pull request Mar 6, 2022
Most of the changes are in SqlStatement::Update part,
- table field is now TableWithJoin type, so I had to take relation field out of it and converted into ObjectName.
- SqlAssignment's id is now Vec<Ident> type, so I took only the first element's value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvements for existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants