Commit 282740a
committed
Add fingerprint pre-check to Recon (Redshift)
## Changes
### What does this PR do?
Adds an opt-in fingerprint pre-check to Recon. When `fingerprint_precheck=True`
and the source has a registered query builder, Recon runs a sketch-based
detection pass (MD5-sub-bucketed aggregates over both sides) before the
row-hash compare pipeline.
- MATCH -> Recon short-circuits in seconds; no full table scan, no JOIN.
- MISMATCH -> an algebraic solver returns the differing row hashes; a
surgical Stage-2 fetch pulls just those rows and feeds them into the
existing `compare.reconcile_data` flow. If the mismatch is systemic
(>15% of sub-buckets), the precheck defers to the existing pipeline.
- Ineligible -> falls through silently.
The flag defaults to False; existing behaviour is unchanged. The algorithm
is byte-identical to the dataprint sketch-based reconciliation library;
this is the first dataprint-into-lakebridge integration. Redshift is the
first dialect — adding Snowflake / Oracle / TSQL is one
`FingerprintQueryBuilder` subclass plus one registry entry.
### Relevant implementation details
- `trigger_recon_service._run_fingerprint_or_reconcile_data` is the single
decision point. Static eligibility centralised in `classify_ineligibility`;
the schema-dependent `unmapped_target_column_mapping` reason is raised
by `align_columns` as a typed exception and routed through
`FingerprintRunMetadata.ineligible(...)`. Every reason maps to an
`IneligibilityReason` enum value and is recorded on
`recon_metrics.fingerprint_metrics.ineligibility_reason`.
- Source-side reads use upstream's `RemoteQueryReader` / `remote_query()`
TVF unmodified; Stage-1 aggregation pushdown verified empirically on a
1 M-row Redshift fixture (DBR 17.3).
- Stage-1 detection is parallelised across source / target via a 2-thread
pool; failure semantics match the serial version.
- Three new fields on `ReconcileConfig`: `fingerprint_precheck`,
`fingerprint_treat_empty_as_null`, `fingerprint_row_count_override`.
- Config version bumps 2 -> 3 with a `v2_migrate` that folds two legacy
spellings (`redshift_fingerprint_precheck`, `use_fingerprint_precheck`)
into the new flag. Existing deployments upgrade automatically.
### Pre-existing fixes that ride along (upstream PR databrickslabs#2339)
Two correctness bugs in the upstream Redshift connector MR (databrickslabs#2339)
surfaced during the dataprint integration P0 / P1 runs against a real
cluster. Both crash the existing row-hash recon path on real customer
schemas and are unrelated to dataprint, but they sat in the integration
path so they are fixed inline. Both fixes live in
`reconcile/query_builder/expression_generator.py` and are pinned by
regression tests in `test_expression_generator.py`.
- **Databricks block missing TIMESTAMP / TIMESTAMPTZ handler.** Redshift's
source-side transform emits `COALESCE(TO_CHAR(ts, 'YYYY-MM-DD
HH24:MI:SS.US'), '_null_recon_')` (always 6 fractional digits), but
the Databricks block had no override, so the target side fell through
to the universal default `TRIM(COALESCE(col, '_null_recon_'))` — Spark
emits a variable-length fractional component, omitted entirely for
zero-microsecond timestamps. The byte-width drift made per-row SHA2
disagree for every TIMESTAMP / TIMESTAMPTZ row in any
Redshift -> Databricks reconcile. Fix: add `COALESCE(DATE_FORMAT(ts,
'yyyy-MM-dd HH:mm:ss.SSSSSS'), '_null_recon_')` so source and target
are byte-identical.
- **Redshift block missing BOOLEAN handler.** The Redshift block defined
overrides only for SUPER / DATE / TIMESTAMP / TIMESTAMPTZ and had no
dialect-level `default`. BOOLEAN columns fell through to the universal
default `TRIM(COALESCE(col, '_null_recon_'))`, which Redshift rejects
during output schema resolution with `function pg_catalog.btrim(boolean)
does not exist`. Any customer schema containing a single BOOLEAN
column crashes row-hash recon end-to-end. Fix: explicit `COALESCE(CASE
WHEN col THEN 'true' WHEN NOT col THEN 'false' ELSE NULL END,
'_null_recon_')` so the rendered string matches Spark's
`cast(boolean AS string)` byte-for-byte.
### Hardening from the internal review round
After the initial internal review on the contributor's fork, three
substantive code changes landed before pushing upstream:
- **Pin TZ-aware Spark target columns to UTC** before formatting in
`fingerprint/spark_target.py`. The Redshift side already pinned UTC via
`TO_CHAR(_ AT TIME ZONE 'UTC', _)`; the Spark side was using
`DATE_FORMAT(ts, _)` which renders in `spark.sql.session.timeZone`. On
a non-UTC cluster the same instant rendered different bytes on the two
sides. Fix splits LTZ vs NTZ handling and routes LTZ through
`TO_UTC_TIMESTAMP(_, CURRENT_TIMEZONE())`. NTZ behaviour unchanged.
- **Stage-2 build failures fall through to the full pipeline** in
`trigger_recon_service.py` instead of marking the table failed. Every
other non-MATCH branch already does this; the `build_mismatch_output`
exception path was the one inconsistency. Metadata still records
`fallback_to_full_pipeline=True` for observability.
- **Cast Redshift strings to `VARCHAR(65535)`** in
`fingerprint/query_builders/redshift.py` instead of bare `VARCHAR`,
whose default 256-byte width truncated long text. `VARCHAR(65535)` is
Redshift's maximum and matches Spark's unbounded string semantics.
Smaller cleanups: dropped unused `ColumnAlignment.exclude_columns`;
reverted a no-op reorder in `connectors/source_adapter.py`; replaced a
flaky wall-clock assertion in `test_fetch_parallel.py` with a
deterministic distinct-thread-id assertion; pinned the exact rendered
SQL on each dialect in `test_expression_generator.py` (instead of
substring-checking two different patterns) and added a regression test
for the Redshift `BOOLEAN` handler.
### Caveats
- DBR 17.3+ required for source-side reads via `remote_query()`
(inherited from upstream's `RemoteQueryReader` adoption).
- MISMATCH-state cost at 1 M scale currently exceeds row-hash-only mode
by 16-94 s because Stage-2 still feeds the existing JOIN. MATCH is
the headline win (38.7% on 1 M rows); billion-row scale is the
production motivation. Stage-1 hash persistence as Stage-2 input is
filed as a follow-up.
- Pre-existing `success_count` formula in `verify_successful_reconciliation`
(upstream PR databrickslabs#2259, commit `e56c79c3d`) is mathematically wrong; sits
next to fingerprint code in `trigger_recon_service.py`. Not fixed
here to keep scope contained; filed separately.
### Tests
- All unit tests on the touched surface pass; `tests/unit/reconcile/`
runs 282 tests in <1 s. The 6 `test_cli_analyze.py` failures are
pre-existing on main and unrelated.
- Regression tests added for every review-round fix (UTC pin, fallback
path, VARCHAR(65535)); `test_expression_generator.py` pins the exact
rendered SQL on each dialect for the two pre-existing fixes.
- Correctness validated end-to-end on a 1 M-row Redshift / Delta fixture
across the 20-scenario dual-mode parity matrix: 39/40 cells PASS, 1
scenario shows a known fingerprint-solver fallback edge with verdict
agreement on both sides — only the cap-bounded `mismatch` count
differs (fingerprint reports the true 10000, normal reports the
cap-50 sample).
- Linter clean: pylint 10.00/10 on touched src; ruff, black, mypy green.
- Integration coverage to follow alongside the recon e2e cluster fixture
(databrickslabs#2453).1 parent 1e02597 commit 282740a
43 files changed
Lines changed: 6941 additions & 35 deletions
File tree
- docs/lakebridge/docs/reconcile
- src/databricks/labs/lakebridge
- reconcile
- fingerprint
- query_builders
- query_builder
- tests/unit
- reconcile
- fingerprint
- query_builder
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
94 | 171 | | |
95 | 172 | | |
96 | 173 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
82 | 82 | | |
83 | 83 | | |
84 | 84 | | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
85 | 97 | | |
86 | 98 | | |
87 | 99 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
280 | 280 | | |
281 | 281 | | |
282 | 282 | | |
283 | | - | |
| 283 | + | |
284 | 284 | | |
285 | 285 | | |
286 | 286 | | |
287 | 287 | | |
288 | 288 | | |
289 | 289 | | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
290 | 304 | | |
291 | 305 | | |
292 | 306 | | |
| |||
314 | 328 | | |
315 | 329 | | |
316 | 330 | | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
317 | 350 | | |
318 | 351 | | |
319 | 352 | | |
| |||
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
Lines changed: 98 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
0 commit comments