Skip to content

[ENG-41614] Introduce TableFormatDetector SPI (Hudi-only) - #190

Merged
dharmendersheshma merged 4 commits into
mainfrom
eng-41614-lakeview-tableformat-spi
May 22, 2026
Merged

[ENG-41614] Introduce TableFormatDetector SPI (Hudi-only)#190
dharmendersheshma merged 4 commits into
mainfrom
eng-41614-lakeview-tableformat-spi

Conversation

@dharmendersheshma

@dharmendersheshma dharmendersheshma commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Sub-PR 1 of 2 splitting #189. Pure refactor — extracts the hardcoded .hoodie/ check in TableDiscoveryService into a TableFormatDetector SPI with one implementation per format. Only HudiTableFormatDetector is registered today, so behavior is unchanged: every Database (including those with no tableFormat in YAML) routes through the Hudi detector.

What changed

New

  • TableFormat enum (HUDI, ICEBERG) with @JsonProperty aliases matching the protobuf enum names (TABLE_FORMAT_HUDI, TABLE_FORMAT_ICEBERG). ICEBERG is unused in this PR but lets the wire shape settle before the iceberg detector + uploader land.
  • TableFormatDetector SPI interface — async matches(path, listedFiles) so future detectors can do follow-up listings without a refactor.
  • HudiTableFormatDetector — same logic as the old isHudiTableFolder (anyMatch(filename startsWith .hoodie)), moved behind the SPI.
  • TestHudiTableFormatDetector — unit coverage.

Modified

  • Database — adds nullable tableFormat field. Null is interpreted as HUDI for backward compatibility with existing YAMLs.
  • TableDiscoveryService — uses Map<TableFormat, TableFormatDetector> and picks the detector by database.getTableFormat() (null → HUDI). The detector map currently has only the HUDI entry.
  • TableDiscoveryServiceTest — constructor signature update.

Hudi safety

Check Status
Existing YAMLs (no tableFormat) still discover Hudi tables ✓ — null → HUDI in detectorFor
HudiTableFormatDetector.matches is byte-for-byte the old isHudiTableFolder
TableMetadataUploaderService (Hudi upload path) untouched
Full :lakeview:test suite passes locally

Out of scope (follow-up PR)

The iceberg-specific pieces from #189 are deferred:

  • IcebergTableFormatDetector + IcebergMetadataUploaderService
  • Database.tableHints + TableHint
  • Table.tableFormat, Table.metadataLocationHint
  • TableDiscoveryAndUploadJob.dispatchUpload routing
  • InitializeTableMetricsCheckpointRequest.tableFormat
  • s3a:// in OBJECT_STORAGE_URI_PATTERN
  • docs/iceberg-support.md

Test plan

  • ./gradlew :lakeview:test green locally (full suite)
  • TableDiscoveryServiceTest (existing) passes unchanged behavior
  • TestHudiTableFormatDetector covers detector contract + iceberg-shape negative case
  • CI green

Extracts the hardcoded `.hoodie/` check in `TableDiscoveryService` into a
`TableFormatDetector` interface with one implementation per format. Only
`HudiTableFormatDetector` is registered today, so behavior is unchanged:
every `Database` (including those with no `tableFormat` set in YAML) is
treated as Hudi.

The new `TableFormat` enum carries `HUDI` and `ICEBERG` values with
`@JsonProperty` aliases for the proto enum names — the latter is unused
in this PR but lets the wire shape settle before the Iceberg detector
and uploader land in the follow-up.

Refactor only. The Iceberg detector + uploader, `Database.tableHints`,
`Table.tableFormat`, `s3a://` URI scheme, and the dispatch-by-format
routing in `TableDiscoveryAndUploadJob` ship in the follow-up PR.
@nimahajan

Copy link
Copy Markdown

Dharmender Sheshma added 3 commits May 22, 2026 11:53
`TableDiscoveryService` gained a `HudiTableFormatDetector` constructor
argument; `LakeviewSyncTool` constructs it directly (no Guice) and was
left out of the previous commit. Pass `new HudiTableFormatDetector()`
to match.
The intermediate `directories` list is only iterated, never modified.
Switching to an unmodifiable collector addresses the SonarQube hint
without depending on `Stream.toList()` (Java 16+) — project still
targets a lower bytecode level.
CI runs on Java 8 where neither `Stream.toList()` (16+) nor
`Collectors.toUnmodifiableList()` (10+) is available. The SonarQube
hint is a non-blocking suggestion; matching the rest of the codebase
takes priority.
@sonarqubecloud

Copy link
Copy Markdown

@dharmendersheshma
dharmendersheshma merged commit 8cfc570 into main May 22, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants