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

Spanner Import/Export INTERLEAVE IN #2128

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jjfox15
Copy link

@jjfox15 jjfox15 commented Jan 12, 2025

Add support for Import/Export of Spanner INTERLEAVE IN tables. This new feature allows for omitting the PARENT keyword in CREATE/ALTER <table> INTERLEAVE IN [PARENT] <parent_table> clauses. INTERLEAVE IN tables are similar to INTERLEAVE IN PARENT, but do not enforce parent/child referential integrity. INTERLEAVE IN tables do not support ON DELETE actions.

I manually verified this works by creating a table with two children, one INTERLEAVE IN and the other INTERLEAVE IN PARENT. All tables had a few rows - and the INTERLEAVE IN table contained a row for which the parent row was missing.

I exported this db to avro using these instructuons, and imported to an empty db following these. I manually checked the target db contained the correct tables and rows, and also verified the entries in the INFORMATION_SCHEMA.Tables were identical across both tables (specifically the INTERLEAVE_TYPE, PARENT_TABLE_NAME, and ON_DELETE_ACTION columns)

…ite/read the type to/from avro. No change to tests yet.
…oDdlConverter. Also properly default to IN PARENT when emitting ddl, in case the interleave type is not set (really only necessary for tests, since otherwise it will always be set.
@jjfox15 jjfox15 marked this pull request as ready for review January 15, 2025 20:40
@jjfox15 jjfox15 requested a review from a team as a code owner January 15, 2025 20:40
Copy link
Contributor

@hengfengli hengfengli left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@darshan-sj darshan-sj left a comment

Choose a reason for hiding this comment

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

Please check If there are Unit tests or Integration tests for the files being modified in this PR. If there are can you add test cases for INTERLEAVE IN ?

darshan-sj
darshan-sj previously approved these changes Jan 24, 2025
Copy link
Contributor

@darshan-sj darshan-sj left a comment

Choose a reason for hiding this comment

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

Please add tests.

Please test Export + import for a PG database with your feature.

Copy link
Contributor

@darshan-sj darshan-sj left a comment

Choose a reason for hiding this comment

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

Please add a test case in ExportPipelineIT

Copy link

codecov bot commented Jan 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.90%. Comparing base (208e2ba) to head (13fc5d4).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2128      +/-   ##
============================================
+ Coverage     46.88%   46.90%   +0.01%     
- Complexity     4011     4018       +7     
============================================
  Files           874      874              
  Lines         52042    52054      +12     
  Branches       5454     5459       +5     
============================================
+ Hits          24398    24414      +16     
+ Misses        25902    25901       -1     
+ Partials       1742     1739       -3     
Components Coverage Δ
spanner-templates 68.83% <100.00%> (+0.04%) ⬆️
spanner-import-export 65.74% <100.00%> (+0.14%) ⬆️
spanner-live-forward-migration 76.50% <ø> (ø)
spanner-live-reverse-replication 78.67% <ø> (ø)
spanner-bulk-migration 87.87% <ø> (ø)
Files with missing lines Coverage Δ
...oud/teleport/spanner/AvroSchemaToDdlConverter.java 84.21% <100.00%> (+0.64%) ⬆️
...va/com/google/cloud/teleport/spanner/AvroUtil.java 93.75% <ø> (ø)
...oud/teleport/spanner/DdlToAvroSchemaConverter.java 93.07% <100.00%> (+0.04%) ⬆️
...a/com/google/cloud/teleport/spanner/ddl/Table.java 89.34% <100.00%> (+0.45%) ⬆️

... and 3 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants