Skip to content

Conversation

@NicolasRicheYopp
Copy link

@NicolasRicheYopp NicolasRicheYopp commented Nov 21, 2025

In the SourceDB -> Spanner flow, add support for Float type primary key for Mysql source.

  • UniformSplitter: Add a boundary extractor and splitter (split the range of value in 2) for Float value

    • splitFloat Implementation

    Because Float is an approximate values, we need additional changes:

    • Add support for approximate value (Float, Double) in isSplittable function. Equals when diff < configurable delta Uphttps://github.com/GoogleCloudPlatform/DataflowTemplates/blob/a73e0f07a719c668396e09de5195e7a9a0b756ce/v2/sourcedb-to-spanner/src/main/java/com/google/cloud/teleport/v2/source/reader/io/jdbc/uniformsplitter/range/Boundary.java#L147
    • The delta mentioned above will be used by default. But if the Mysql column has a specific precision - FLOAT(size, d) - d being the number of decimals. This granularity should carry over as delta to use. We need to add a 'delta' value in PartitionColumn
    • Tests
  • Source IndexType: Add a new IndexType.Float which map internally to Java Float.class

    • Implementation
    • Tests
  • MysqlDialectAdapter: Map Mysql source FLOAT column type to the newly created IndexType above

    • Implementation
    • Tests
  • Add Integration tests in v2/sourcedb-to-spanner/src/test/resources/DataTypesIT

@google-cla
Copy link

google-cla bot commented Nov 21, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.


private static Float splitFloats(Float start, Float end) {
if (start == null && end == null) {
return null;
Copy link
Author

@NicolasRicheYopp NicolasRicheYopp Nov 21, 2025

Choose a reason for hiding this comment

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

BoundarySplitter interface comments does not specify what should be the behavior when both start and end parameters are null :

  • Implementations must not assume that that start is less than end. It depends on the
    specific ordering used by the database schema.
  • Implementations must be overflow safe.
  • Implementations must guarantee that the splitter is idempotent. The same boundary must get
    same split point through the lifetime of the migration.
  • Implementations must guarantee that the database would teat the splitpoint as being
    in-between the start and end as per the ordering and collation used for the partition
    column.

For consistency, I then followed the behavior used by other split functions:
When both start and end params are null the split functions return null.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's right.


// If signs are different, simple addition is safe from overflow
// because the values cancel each other out towards zero.
if ((start < 0 && end > 0) || (start > 0 && end < 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For float and double we also need to enhance isSplittable to stop the splitting at appropriate precision.

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.36%. Comparing base (ef38a80) to head (ad36de1).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3016      +/-   ##
============================================
+ Coverage     50.25%   55.36%   +5.10%     
+ Complexity     5021     1623    -3398     
============================================
  Files           967      465     -502     
  Lines         59261    26060   -33201     
  Branches       6458     2734    -3724     
============================================
- Hits          29783    14427   -15356     
+ Misses        27374    10760   -16614     
+ Partials       2104      873    -1231     
Components Coverage Δ
spanner-templates 71.35% <100.00%> (+0.90%) ⬆️
spanner-import-export ∅ <ø> (∅)
spanner-live-forward-migration 79.69% <ø> (ø)
spanner-live-reverse-replication 77.04% <ø> (-0.04%) ⬇️
spanner-bulk-migration 88.36% <100.00%> (+0.03%) ⬆️
Files with missing lines Coverage Δ
...jdbc/dialectadapter/mysql/MysqlDialectAdapter.java 99.61% <100.00%> (+<0.01%) ⬆️
...niformsplitter/range/BoundaryExtractorFactory.java 100.00% <100.00%> (ø)
...uniformsplitter/range/BoundarySplitterFactory.java 100.00% <100.00%> (ø)
...source/reader/io/schema/SourceColumnIndexInfo.java 68.00% <100.00%> (+1.33%) ⬆️

... and 522 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants