added oracle support to data validation - #4173
Conversation
a50d3d0 to
c60c426
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 support for Oracle data types within the data validation template. By extending the visitor pattern to handle Float32 values and implementing robust unit and integration tests, the changes ensure that data validation remains accurate and consistent across different data types during migration processes. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for the FLOAT32 data type in the unified visitor and hasher classes, and adds comprehensive integration and unit tests validating Oracle data type mappings, including a new smoke test (GCSSpannerDVOracleSmokeIT). Feedback on the changes suggests removing an unnecessary Thread.sleep delay in the integration test since Dataflow provisioning time is sufficient to satisfy the staleness window, and updating the dispatch logic in IUnifiedVisitor to handle PG_UUID using value.getUuid() instead of value.getString().
| // Wait for Spanner's 20-second exact staleness read bound in SpannerReaderTransform | ||
| Thread.sleep(20000); |
There was a problem hiding this comment.
In template integration tests that perform stale reads (such as Spanner's exact staleness read bound), there is no need to add a delay (like Thread.sleep) before executing the pipeline when running on Dataflow. The setup and provisioning time of the Dataflow runner is more than sufficient to satisfy the staleness window. Removing this sleep will speed up the integration test execution.
References
- When writing integration tests that perform stale reads (such as Spanner's exact staleness read bound), do not add a delay (e.g., Thread.sleep) before executing the pipeline if the test runs using DataflowRunner, as the runner's setup time is long enough to satisfy the staleness window.
| case UUID -> visitor.visitUuid(value.getUuid()); | ||
| case INT64 -> visitor.visitInt64(value.getInt64()); | ||
| case FLOAT64 -> visitor.visitFloat64(value.getFloat64()); | ||
| case FLOAT32 -> visitor.visitFloat32(value.getFloat32()); |
There was a problem hiding this comment.
When retrieving UUID or PG_UUID values from Cloud Spanner Value objects, we should use value.getUuid() instead of value.getString() as UUID values are received as UUID objects. Please update the UUID case in the dispatch switch to also handle PG_UUID:
case UUID, PG_UUID -> visitor.visitUuid(value.getUuid());References
- When retrieving UUID or PG_UUID values from Cloud Spanner Value objects, use value.getUuid() instead of value.getString() as UUID values are received as UUID objects.
Added support for oracle validations in the data validation template.