Support max_sample_size via sampling_options with engine-aware bounds

[ganeshdogiparthi-db]

Changes

What does this PR do?

Adds a sampling_options block to the reconcile table config so users can control how many sample rows are captured per result set (mismatch, missing-in-source,
missing-in-target) instead of the fixed 50. Previously large sample sizes on Databricks blew up the UNION-based sampling query against Spark Connect; the Databricks path now uses
a temp view so it scales.

Relevant implementation details

• SamplingSpecifications now defaults to count / 50 and validates the value (coerces to int, rejects bools/non-numeric, keeps fraction disabled).
  Table.get_max_sample_size() exposes the configured value.
• Sample-size bounds are applied in NormalizeReconConfigService (engine-aware), not in the samplers:
  • Databricks source: clamped to [1, 50_000].
  • Non-Databricks source: honored up to 400, larger values capped at 400 with a warning.
  • value <= 0 falls back to 50.
• SamplingQueryBuilder registers the recon keys as a temp view for the Databricks engine (SELECT * FROM recon_keys_<uuid>) so the generated SQL stays small regardless of
  sample size; other dialects keep the inline-UNION derived table.
• The old fixed [50, 400] clamp was removed from the samplers.

Caveats/things to watch out for when reviewing:

• Non-Databricks sources are intentionally capped at 400 (matches prior behaviour); only Databricks honors larger sizes up to 50,000.
• fraction sampling type stays disabled and raises a config error.

Linked issues

Resolves #2559

Functionality

• added relevant user documentation
• added new CLI command
• modified existing command: databricks labs lakebridge ...
• new config field: sampling_options on the reconcile Table config

Tests

• manually tested — end-to-end on Databricks source (default → 50, 100000 → capped to 50000) and Redshift source (default → 50, 500 → capped to 400 with warning), covering
  column mapping, drop columns and schema validation
• added unit tests
• added integration tests

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[codecov]

Codecov Report

❌ Patch coverage is 90.16393% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.28%. Comparing base (d2fdae7) to head (f0871e7).

Files with missing lines	Patch %	Lines
...labs/lakebridge/reconcile/trigger_recon_service.py	78.57%	3 Missing ⚠️
...rc/databricks/labs/lakebridge/reconcile/sampler.py	33.33%	2 Missing ⚠️
...bricks/labs/lakebridge/reconcile/reconciliation.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2497      +/-   ##
==========================================
+ Coverage   69.41%   70.28%   +0.86%     
==========================================
  Files         106      106              
  Lines        9578     9607      +29     
  Branches     1056     1060       +4     
==========================================
+ Hits         6649     6752     +103     
+ Misses       2731     2650      -81     
- Partials      198      205       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

✅ 174/174 passed, 7 flaky, 2 skipped, 1h0m17s total

Flaky tests:

• 🤪 test_installs_and_runs_pypi_bladebridge (21.987s)
• 🤪 test_transpiles_informatica_to_sparksql_non_interactive[True] (21.768s)
• 🤪 test_transpiles_informatica_to_sparksql_non_interactive[False] (21.778s)
• 🤪 test_transpile_teradata_sql_non_interactive[True] (5.716s)
• 🤪 test_transpile_teradata_sql_non_interactive[False] (7.381s)
• 🤪 test_transpiles_informatica_to_sparksql (23.237s)
• 🤪 test_transpile_teradata_sql (24.385s)

_{Running from acceptance #5034}

[bishwajit-db]

Thanks for the PR. The approach looks good overall: centralizing the bounds at normalization and switching the Databricks path to a temp view both make sense, and the test coverage is solid. A few comments inline, one of which is a real (if low-severity) mutation bug in the normalize service.

[bishwajit-db]

LGTM

[BesikiML]

Thanks for the update. Overall this looks solid, and docs/tests coverage is strong.

I have one concern:

Potential issue: Databricks temp-view cleanup in sampling path

In SamplingQueryBuilder (Databricks path), each sampling query now creates a unique temp view (recon_keys_<uuid>) via createOrReplaceTempView(...).

I checked the reconcile flow and don’t see a corresponding drop/cleanup for those views after query execution.

Because the view name is unique per call, this can accumulate temp views during longer runs (many tables and multiple sampling passes), which may increase Spark session metadata/memory overhead.

Suggestion: track the created view name and explicitly drop it after the read step (or wrap create/query/drop in a scoped helper).

[m-abulazm]

Unblocking although the UX still needs improvement

unblocking

[BesikiML]

1. Aggregate reconcile ignores sampling_options
   reconcile_data threads max_sample_size=table_conf.get_max_sample_size(), but reconcile_agg_data_per_rule still hardcodes DEFAULT_SAMPLE_ROWS:

missing_in_src=missing_in_src.limit(DEFAULT_SAMPLE_ROWS),
missing_in_tgt=missing_in_tgt.limit(DEFAULT_SAMPLE_ROWS),

A user setting sampling_options: { specifications: { value: 500 } } on an aggregates table gets 500 for row-level reconcile but 50 for aggregates-reconcile. Either wire get_max_sample_size() here or document aggregates as a known limitation.

2. Temp-view cleanup resolved
   My earlier concern about recon_keys_ views accumulating is addressed by _drop_sample_temp_views() in TriggerReconService, called per table in a finally block. Scoped to our prefix via SHOW VIEWS, looks good.

3. Silent lower-bound fallback
   value <= 0 silently resets to 50 in SamplingSpecifications.post_init, while upper-bound capping logs a WARNING. Consider a warning on the lower bound too for consistency (acknowledged m-abulazm's UX note).

4. PR hygiene (nit)
   Resolves #.. in the PR body should link a real issue or be removed.
   SUPPORTED_OPERATIONS / discover-tables constants in recon_config.py look like merge artifacts, unrelated to sampling.

5. Test coverage gap (nit)
   Codecov shows _drop_sample_temp_views (~12 lines in trigger_recon_service.py) largely untested. A small unit test mocking spark.sql("SHOW VIEWS") would close that gap.

Happy to approve once aggregate sampling is either wired or explicitly documented.

[ganeshdogiparthi-db]

• A user setting sampling_options: { specifications: { value: 500 } } on an aggregates table gets 500 for row-level reconcile but 50 for aggregates-reconcile. Either wire get_max_sample_size() here or document aggregates as a known limitation.

  • Actually, the sampling logic is never used for aggregate reconciliation, so I documented it as a known limitation.

• Silent lower-bound fallback
  value <= 0 silently resets to 50 in SamplingSpecifications.post_init, while upper-bound capping logs a WARNING. Consider a warning on the lower bound too for consistency (acknowledged m-abulazm's UX note).

  • Done. Also added this in test case.

• PR hygiene (nit)
  Resolves #.. in the PR body should link a real issue or be removed.
  SUPPORTED_OPERATIONS / discover-tables constants in recon_config.py look like merge artifacts, unrelated to sampling.

  • Done.

• Test coverage gap (nit)
  Codecov shows _drop_sample_temp_views (~12 lines in trigger_recon_service.py) largely untested. A small unit test mocking spark.sql("SHOW VIEWS") would close that gap.

  • There was an issue with our process. The drop_views function I added was private. If I wrote a test for the private function, the linter complained. If I didn't write a test, the code coverage check failed. To avoid both issues, I moved the function out of the class, made it public, and added a test for it.