Skip to content

Expose TableFragmentType via a Replace flag #74

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

Merged
merged 6 commits into from
Feb 15, 2022
Merged

Expose TableFragmentType via a Replace flag #74

merged 6 commits into from
Feb 15, 2022

Conversation

w1ndy
Copy link
Contributor

@w1ndy w1ndy commented Feb 9, 2022

Pull Request Description

This PR proposes a fix for #73 . Table fragments of type DataReplace should cause the existing result set to be replaced with new one; otherwise, the obtained query result may contain duplicated entries. To make users aware of this, this PR adds a flag called Replace in struct Row to indicate that the existing result set should be emptied before adding this row to the set.


Future Release Comment

Breaking Changes:

  • The new Replace flag in struct Row should be handled to avoid duplicated entries in the result set. Adding the following code to handle the Replace flag in the iter.Do calls is recommended:

    var results []Result
    iter.Do(
        func(row *table.Row) error {
      	  // result := ...
      	  if row.Replace {
      		  results = results[:0]
      	  }
      	  results = append(results, result)
      	  return nil
        },
    )

Fixes:

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2022

Codecov Report

Merging #74 (e1a211d) into master (02c5c0e) will increase coverage by 2.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
+ Coverage   50.29%   52.35%   +2.05%     
==========================================
  Files          28       30       +2     
  Lines        4096     4273     +177     
==========================================
+ Hits         2060     2237     +177     
+ Misses       1890     1877      -13     
- Partials      146      159      +13     
Impacted Files Coverage Δ
kusto/data/table/table.go 32.25% <ø> (ø)
kusto/reader.go 65.24% <100.00%> (ø)
kusto/statemachine.go 72.61% <100.00%> (ø)
kusto/mock.go 50.00% <0.00%> (-13.77%) ⬇️
kusto/test/etoe/etoe_env.go 42.00% <0.00%> (-6.00%) ⬇️
kusto/ingest/internal/filesystem/filesystem.go 26.80% <0.00%> (-1.12%) ⬇️
kusto/ingest/streaming.go 63.21% <0.00%> (ø)
kusto/ingest/file_options.go 25.89% <0.00%> (ø)
kusto/ingest/internal/conn/conn.go 66.10% <0.00%> (+0.36%) ⬆️
kusto/ingest/result.go 6.95% <0.00%> (+6.95%) ⬆️
... and 2 more

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 b89f168...e1a211d. Read the comment docs.

@AsafMah
Copy link
Contributor

AsafMah commented Feb 14, 2022

Overall looks good, just needs a test in statemachine_test.go

@w1ndy
Copy link
Contributor Author

w1ndy commented Feb 14, 2022

@AsafMah Thanks for your review! I've added a simple test to statemachine_test.go. Could you help me review again?

@w1ndy
Copy link
Contributor Author

w1ndy commented Feb 15, 2022

@AsafMah Good suggestion, I have added the test

Copy link
Contributor

@AsafMah AsafMah left a comment

Choose a reason for hiding this comment

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

🚀

@AsafMah AsafMah merged commit 9a620e9 into Azure:master Feb 15, 2022
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.

3 participants