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 insert support #32

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

skuehn
Copy link
Contributor

@skuehn skuehn commented Aug 28, 2021

This is another WIP draft of partial DML support. The column->extractor selection is broken due to tables resolving to their names rather than the query type $query/$aggregate/etc.... This patch removes the extractor filtering, so all extractors will always run. I'm not sure if this is inefficient, b/c the visitors of irrelevant extractors don't seem to be invoked. Any thoughts appreciated.

@codecov
Copy link

codecov bot commented Aug 28, 2021

Codecov Report

Merging #32 (cae8e7b) into master (4685db1) will increase coverage by 0.03%.
The diff coverage is 96.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #32      +/-   ##
============================================
+ Coverage     89.35%   89.39%   +0.03%     
+ Complexity      266      264       -2     
============================================
  Files            42       43       +1     
  Lines          1137     1150      +13     
  Branches         64       64              
============================================
+ Hits           1016     1028      +12     
  Misses           86       86              
- Partials         35       36       +1     
Impacted Files Coverage Δ
...loud/solutions/datalineage/model/QueryColumns.java 50.00% <ø> (-25.00%) ⬇️
...atalineage/extractor/InsertStatementExtractor.java 95.23% <95.23%> (ø)
...cloud/solutions/datalineage/BigQuerySqlParser.java 97.72% <100.00%> (+0.10%) ⬆️
...neage/extractor/ColumnLineageExtractorFactory.java 89.65% <100.00%> (-1.78%) ⬇️
...s/datalineage/extractor/OutputColumnExtractor.java 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 4685db1...cae8e7b. Read the comment docs.

*/
public ImmutableSet<ColumnLineageExtractor> buildExtractors() {
return outputColumns()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The extractor types and ordering is still necessary - this has just been temporarily disabled. One question is how extractors work in order (or nested?). An insert with SET col = CONCAT(colA, colB) will be parsed correctly with colA and colB properly set as parents of col, but the evaluation occurs after the DML statement evaluation and the type is a '$insert'. I think this just needs some extension to allow the ordering of extractors to change, as well as allowing an extractor registered on the table itself instead of columnType (I think this is necessary for DML, but still wrapping my head around it). Any advice appreciated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They work in order

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.

None yet

2 participants