Repository navigation
Add Oracle Live datatype ITs - #4492
dhwanilpatel wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4492 +/- ##
=============================================
+ Coverage 40.97% 62.48% +21.51%
- Complexity 741 3776 +3035
=============================================
Files 210 600 +390
Lines 15359 36598 +21239
Branches 1628 4146 +2518
=============================================
+ Hits 6294 22870 +16576
- Misses 8545 12567 +4022
- Partials 520 1161 +641
🚀 New features to boost your workflow:
|
28893cb to
00814d3
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust integration testing framework for verifying Oracle data type replication to Spanner. It covers a wide range of data types and mapping scenarios, ensuring data integrity across both backfill and CDC replication paths for both GoogleSQL and PostgreSQL-dialect Spanner databases. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive integration tests for replicating Oracle data types to both GoogleSQL and PostgreSQL-dialect Spanner databases using the Datastream-to-Spanner template. It includes test data, test cases, and DDL schemas for both dialects. The review feedback highlights a critical syntax error in the GoogleSQL Spanner schema file, where trailing commas in the table definitions will cause DDL execution to fail.
| * <p><b>LOB validation is disabled (6 tables, {@link #LOB_TABLES_NOT_VALIDATED}: {@code | ||
| * blob_table}, {@code blob_to_str_table}, {@code clob_table}, {@code clob_to_bytes_table}, {@code | ||
| * nclob_table}, {@code nclob_to_bytes_table})</b>: the test does not enable Datastream | ||
| * large-object streaming ({@code OracleSourceConfig.stream_large_objects}), and without it |
There was a problem hiding this comment.
why is this not enabled on datastream ? If we support it we should test it
There was a problem hiding this comment.
This will need change in common folder in it/google-cloud-platform package for enabling streaming for datastream. I will make that change as a followup after this PR and enable this validations.
|
|
||
| @AfterClass | ||
| public static void cleanUp() { | ||
| cleanUpSharedResources(testInstances); |
There was a problem hiding this comment.
please check if the jobs are killed after the test completes or after all the tests are successful. It is preferable to cleanup resources after the test rather than waiting for all the tests - where possible.
There was a problem hiding this comment.
There are some shared resources and some test specific resource. I will divide those and clean test specific resources [dataflow/datastream] after test and clean shared resource [pubsub/gcs] in afterClass.
| @@ -0,0 +1,1086 @@ | |||
| CREATE TABLE "varchar2_table" ("id" NUMBER(10) NOT NULL, "varchar2_col" VARCHAR2(4000), PRIMARY KEY ("id")); | |||
There was a problem hiding this comment.
nit: This dataset - and the others can be used across live and bulk. Can you evaluate if that is possible - to avoid duplication and ensure coverage ?
There was a problem hiding this comment.
We can explore that, but a couple of things to highlight:
- Expected values differ between bulk and live even for the same data (JDBC vs Datastream output), so each IT would still keep its own expectations. We'd mostly save one DDL file.
- Live has some Datastream and template limitations (e.g. unsupported types, and TIMESTAMP WITH TIME ZONE restricted to UTC values), so sharing the dataset could reduce bulk's coverage too.
Add Oracle Live datatype IT