Skip to content

[BEAM-11277] Add method to check if two TableSchema are equal - #14394

Merged
pabloem merged 3 commits into
apache:masterfrom
chunyang:cyang/copy-schema-relaxation
Apr 7, 2021
Merged

[BEAM-11277] Add method to check if two TableSchema are equal#14394
pabloem merged 3 commits into
apache:masterfrom
chunyang:cyang/copy-schema-relaxation

Conversation

@chunyang

Copy link
Copy Markdown
Contributor

A followup to #14113 to improve the schema equality checking.

The check in the previous PR has a high false positive rate for detecting whether a schema update job needs to be run because a schema update job is created even if the field ordering is different.

This PR adds a schema equivalence check with more specificity. Now we can do a field-order insensitive comparison on the schema and only run a schema update job when the fields themselves are different.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

LangSDKULRDataflowFlinkSamzaSparkTwister2
Go------Build Status---Build Status---
JavaBuild StatusBuild StatusBuild Status
Build Status
Build Status
Build Status
Build StatusBuild Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build StatusBuild Status
Build Status
Build Status
Build Status
PythonBuild Status
Build Status
Build Status
---Build Status
Build Status
Build Status
Build Status
Build Status
---Build Status---
XLangBuild Status---Build StatusBuild Status---Build Status---

Pre-Commit Tests Status (on master branch)

---JavaPythonGoWebsiteWhitespaceTypescript
Non-portableBuild Status
Build Status
Build Status
Build Status
Build Status
Build StatusBuild StatusBuild StatusBuild Status
Portable---Build Status------------

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@chunyang

Copy link
Copy Markdown
ContributorAuthor

R: @pabloem

Looks like I just missed the closing of the last PR :)

@pabloem

Copy link
Copy Markdown
Member

ooops - sorry about the surprise. I know of someone else making a PR to the same file, so I wanted to avoid merge issues.
I'll take a look at this in a bit.

@codecov

codecovBot commented Mar 31, 2021

Copy link
Copy Markdown

Codecov Report

Merging #14394 (abbf09e) into master (93a6927) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #14394 +/- ##
==========================================
- Coverage 83.45% 83.44% -0.02% 
==========================================
Files 470 470 Lines 59001 59100 +99 ==========================================
+ Hits 49239 49315 +76 - Misses 9762 9785 +23 
Impacted FilesCoverage Δ
...s/python/apache_beam/io/gcp/bigquery_file_loads.py86.60% <0.00%> (-4.51%)⬇️
...hon/apache_beam/runners/direct/test_stream_impl.py91.91% <0.00%> (-2.21%)⬇️
...apache_beam/runners/dataflow/internal/apiclient.py73.80% <0.00%> (-0.66%)⬇️
...s/sdks/python/apache_beam/io/gcp/bigquery_tools.py87.93% <0.00%> (-0.17%)⬇️
...sdks/python/apache_beam/transforms/environments.py87.85% <0.00%> (-0.08%)⬇️
...ld/srcs/sdks/python/apache_beam/io/gcp/bigquery.py75.52% <0.00%> (ø)
...uild/srcs/sdks/python/apache_beam/io/gcp/pubsub.py92.15% <0.00%> (+0.03%)⬆️
...ache_beam/runners/interactive/recording_manager.py98.96% <0.00%> (+0.03%)⬆️
...38/build/srcs/sdks/python/apache_beam/io/iobase.py84.98% <0.00%> (+0.17%)⬆️
...he_beam/runners/interactive/pipeline_instrument.py99.47% <0.00%> (+0.26%)⬆️
... and 7 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 dcd8778...abbf09e. Read the comment docs.

@chunyang

Copy link
Copy Markdown
ContributorAuthor

Thanks, no worries! Busy end of quarter so I haven't had a lot of time to look at this.

@chunyang

chunyang commented Mar 31, 2021

Copy link
Copy Markdown
ContributorAuthor

Hmm, I can't figure out why that doc error is happening. nvm, figured it out.

@chunyang
chunyangforce-pushed the cyang/copy-schema-relaxation branch from ff5a091 to a7990e7CompareMarch 31, 2021 23:14
@pabloem

Copy link
Copy Markdown
Member

Run Python 3.8 PostCommit

1 similar comment
@pabloem

Copy link
Copy Markdown
Member

Run Python 3.8 PostCommit

@pabloem

Copy link
Copy Markdown
Member

LGTM thanks @chunyang !

@pabloem
pabloem merged commit 79546be into apache:masterApr 7, 2021
@chunyang
chunyang deleted the cyang/copy-schema-relaxation branch April 7, 2021 20:04
Sign up for freeto 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

@chunyang@pabloem