Disallow enabling RTAS alongside WAP or replication by mkuchenbecker · Pull Request #656 · linkedin/openhouse

mkuchenbecker · 2026-07-23T21:36:58Z

Summary

RTAS (replace.enabled) could be enabled on a table that also had WAP (write.wap.enabled) or
replication configured. RTAS is incompatible. This stops them from being enabled together. Its implemented as a generic "these features are incompatible" set of tuples so defining new exclusions or cleaning them up is cheap / easy.

Fix

Don't allow enabling RTAS in conjunction with WAP and replication at the same time.

Reject, on both create and update, any request whose resulting metadata would enable RTAS alongside
WAP or replication (OpenHouseInternalRepositoryImpl.validateFeatureCompatibility, called at the top
of save).

Testing Done

Integration (black-box SQL, real embedded server + Spark): RtasWapExclusivityTest
- enabling replace.enabled on a WAP table is rejected,
- enabling write.wap.enabled on an RTAS table is rejected,
- creating a table with both RTAS and WAP is rejected,
- enabling replace.enabled on a table with a replication policy is rejected.
Full catalogTest suite green; Spotless clean.

RTAS (replace.enabled) and WAP (write.wap.enabled) are mutually exclusive — a staged WAP write and a whole-table replace do not compose. The replace path already refused to run RTAS while WAP was enabled, but nothing stopped a table from having both flags enabled. Reject, on create and update, any request whose resulting table properties would enable both. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Extend the RTAS compatibility guard so a table cannot enable RTAS (replace.enabled) while replication is configured, mirroring the RTAS/WAP exclusivity. Running RTAS on a replicated table was already blocked at replace time; this closes the enable-time gap on create and update. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…essage Addresses review feedback on the RTAS/WAP/replication compatibility check: - Rename the exception operation INCOMPATIBLE_TBLPROPS -> INCOMPATIBLE_FEATURES; the check is about incompatible features, not just table properties. - Generalize validateFeatureCompatibility: instead of RTAS-specific hardcoded branches, declare mutually-exclusive feature pairs (each a label + an "is-enabled" predicate) in a list and iterate. Declaring a new incompatibility is now just adding a pair. - Make the rejection message generic and actionable: "cannot enable X while Y is enabled. Disable Y to enable X." — instead of "they are mutually exclusive". - isReplicationConfigured now takes Optional<Policies> (no nullable arg), drops the needless fully-qualified type name, and is reused by validateReplaceTable (removing the duplicated inline replication check). - Add a focused unit test for isReplicationConfigured covering empty/null/empty- config vs a configured destination. Testing: new OpenHouseInternalRepositoryImplTest cases pass; the black-box RtasWapExclusivityTest (all 4) passes against the new message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Follow-up to review: replace the List<Feature[]> ad-hoc array pairs with a proper IncompatibleFeatures tuple type (feature, conflictsWith), so the declaration reads as a list of tuples and the validation loop is fully generic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…n failure The mutual-exclusivity check now throws RequestValidationFailureException instead of UnsupportedClientOperationException. ALTER/CREATE are supported operations; it's the requested property combination that's invalid, so a request-validation failure is the accurate semantic — and it still maps to HTTP 400. Note: a raw IllegalArgumentException/IllegalStateException cannot be used here. validateFeatureCompatibility runs inside the @Repository-proxied save(), so Spring's persistence-exception translation (EntityManagerFactoryUtils.convertJpaAccessExceptionIfPossible) would rewrap those into InvalidDataAccessApiUsageException, which is unhandled and surfaces as HTTP 500 -> the client maps 500 to CommitStateUnknownException (a system error). RequestValidationFailureException is a plain domain exception that is not translated, so it reaches its 400 handler. Removes the now-unused INCOMPATIBLE_FEATURES operation. Verified: black-box RtasWapExclusivityTest (all 4) still returns BadRequestException (400). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@repository

Enable/disable feature-compatibility (a table cannot enable RTAS alongside WAP or replication) is argument validation: it depends only on the requested table metadata, not on persisted state. Move it out of the repository's save() path and into OpenHouseTablesApiValidator, where request-only validation belongs and where it runs in the handler layer before the @repository proxy. This also sidesteps Spring's persistence-exception translation: exceptions thrown inside the @repository save() path are run through PersistenceExceptionTranslationPostProcessor, which wraps IllegalArgument/ IllegalState into InvalidDataAccessApiUsageException (unhandled -> HTTP 500 -> client CommitStateUnknownException). Validating in the API layer keeps the rejection a clean 400. The replace-time gate in validateReplaceTable stays in the repository: it is state validation that inspects the persisted table (replace.enabled opt-in plus current WAP/replication state) and cannot be evaluated from the request alone. Its WAP/replication branch is kept as defense-in-depth and becomes the durable protection once the replace.enabled flag is removed at GA. Enable-time rejection is covered by new TablesValidatorTest cases and the black-box RtasWapExclusivityTest. The two e2e tests that assumed RTAS could coexist with WAP/replication at create time are removed, since the enable-time validator now forbids creating that state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…eCompatibility Make validateFeatureCompatibility generic over the mutual-exclusivity rules by taking the List<IncompatibleFeatures> as a parameter instead of reading the static MUTUALLY_EXCLUSIVE_FEATURES field directly. Both call sites pass the MUTUALLY_EXCLUSIVE_FEATURES literal, so the rule set is still declared once and the method no longer depends implicitly on a global. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mkuchenbecker · 2026-08-03T20:25:17Z

    RequestAndValidateHelper.deleteTableAndValidateResponse(mvc, table);
  }

-  @SneakyThrows


Leave these tests. Why you removed them is beyond me. Defense in depth, we added API validation which is handlebars on this function. DO not expand the problem beyond the description. DRY does not apply with respect to validation.

…toggle This PR is scoped to add net-new enable-time validation that rejects enabling RTAS alongside WAP or replication. It must not change existing validation or existing tests. Two earlier changes had drifted out of that scope and are reverted here so the existing code and tests match main byte-for-byte: - OpenHouseInternalRepositoryImpl.validateReplaceTable is restored (the extracted isReplicationConfigured helper and the reworded comment are removed). The replace-time WAP/replication gate stays exactly as it was on main. - The two existing e2e tests testReplaceWithWapEnabledIsRejected and testReplaceWithReplicationEnabledIsRejected, and the helper's unit tests in OpenHouseInternalRepositoryImplTest, are restored rather than deleted. Those two existing e2e tests create a table with RTAS and WAP (or replication) enabled together so they can exercise the replace-time gate. The net-new enable-time validation would reject that setup at create. To let existing behavior and its tests stand while still shipping the net-new validation, add a single configuration toggle, cluster.tables.feature-compatibility-validation-enabled, which defaults to true. When it is set to false the enable-time feature-compatibility check is skipped, which is an escape hatch that allows creating a table with otherwise mutually-exclusive features so downstream behavior can be tested. The existing e2e test class sets this property to false; the net-new validation remains on by default everywhere else and is covered by TablesValidatorTest and the black-box RtasWapExclusivityTest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mkuchenbecker · 2026-08-04T18:39:31Z

+  // allow requests that enable otherwise mutually-exclusive features, e.g. to exercise downstream
+  // behavior that can only be reached once such a table exists.
+  @Value("${cluster.tables.feature-compatibility-validation-enabled:true}")
+  private boolean featureCompatibilityValidationEnabled;


This should be a table property not a cluster property.

Replace the cluster-level configuration toggle with a per-table property. A cluster property disables the enable-time feature-compatibility validation for the entire cluster, which is too broad. A table property scopes the opt-out to the specific table, matching how the features themselves (replace.enabled for RTAS, write.wap.enabled for WAP) are expressed as table properties. The validation is enabled unless a table sets feature.compatibility.validation.enabled to false. When it is false the enable-time check that rejects enabling RTAS alongside WAP or replication is skipped for that table, which lets a table be created with those otherwise mutually-exclusive features enabled so behavior that only such a table can reach can be exercised. The property is a regular writable table property, so it defaults to validating when absent. Revert the cluster property: ClusterProperties is restored to its previous state, and the validator no longer depends on it. The two existing e2e tests testReplaceWithWapEnabledIsRejected and testReplaceWithReplicationEnabledIs Rejected set this table property to false so they can create a table with the incompatible features enabled and exercise the replace-time gate, replacing the class-level test property override used before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mkuchenbecker · 2026-08-04T19:26:25Z

+  public static final String FEATURE_COMPATIBILITY_VALIDATION_ENABLED_TABLE_PROP =
+      "feature.compatibility.validation.enabled";


Reverse this to disabled. This is rolling out default enabled and you must explicitly disable validation. Either that or we need a clear default resolution.

…perty Rename the escape-hatch table property to feature.compatibility.validation.disabled. The feature-compatibility validation rolls out enabled by default, and a table must explicitly set this property to true to opt out, so an absent property always means the validation runs. This reads as an intentional opt-out rather than relying on the default value of an "enabled" flag. Reuse the existing isTablePropEnabled helper: validation is skipped only when the property is present and true. Drop the previous helper that defaulted an "enabled" flag to true. The two existing e2e tests set the property to true to create tables with the incompatible features enabled and reach the replace-time gate. The comments that justified those test changes are removed, since comments should describe the code rather than the history of the change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mkuchenbecker and others added 2 commits July 23, 2026 14:36

mkuchenbecker changed the title ~~Disallow enabling RTAS and WAP on the same table~~ Disallow enabling RTAS alongside WAP or replication Jul 23, 2026

Merge branch 'main' into mkuchenbecker/r7-block-rtas-wap

4fd8acb

mkuchenbecker commented Jul 27, 2026

View reviewed changes

Comment thread ...c/main/java/com/linkedin/openhouse/common/exception/UnsupportedClientOperationException.java Outdated

mkuchenbecker commented Jul 27, 2026

View reviewed changes