Fix SchemaUtils to surface actual validation failure reason

[Akanksha-kedia]

Description

The updateSchema endpoint in PinotSchemaRestletResource was catching SchemaBackwardIncompatibleException but discarding the actual error details, returning only a generic message: "Backward incompatible schema <name>. Only allow adding new columns".

This made it very difficult for users to understand why their schema update was rejected. The exception itself already contains detailed information about incompatible field specifications, missing columns, primary key changes, and suggestions for fixing the issue — but this information was never surfaced to the user.

After this fix, the error response includes the actual reason from e.getMessage(), giving users actionable information about what is incompatible and how to fix it.

Related Issue

Fixes #14787

Changes Made

• Modified PinotSchemaRestletResource.updateSchema() to include e.getMessage() in the error response for SchemaBackwardIncompatibleException, replacing the generic "Only allow adding new columns" message.
• Added a test assertion in PinotSchemaRestletResourceTest to verify that backward-incompatible schema update errors include the actual incompatibility details.

Testing Done

• Unit test updated to verify error message includes incompatibility details
• Spotless formatting passes
• Checkstyle passes
• Compilation verified

Upgrade Notes

None. This is a user-facing error message improvement only — no API or configuration changes.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.81%. Comparing base (7fe517a) to head (200057c).
⚠️ Report is 299 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18896      +/-   ##
============================================
+ Coverage     63.68%   64.81%   +1.12%     
+ Complexity     1684     1347     -337     
============================================
  Files          3262     3393     +131     
  Lines        199826   211664   +11838     
  Branches      31031    33305    +2274     
============================================
+ Hits         127264   137193    +9929     
- Misses        62414    63390     +976     
- Partials      10148    11081     +933     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-21	64.81% <100.00%> (+1.12%)	⬆️
temurin	64.81% <100.00%> (+1.12%)	⬆️
unittests	64.81% <100.00%> (+1.12%)	⬆️
unittests1	56.99% <ø> (+1.23%)	⬆️
unittests2	37.18% <100.00%> (+2.20%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Akanksha-kedia]

cc @Jackie-Jiang @siddharthteotia @npawar — requesting your review.

What this PR does

Fixes a UX paper-cut in schema validation: when SchemaUtils rejects a backward-incompatible schema change, the REST response previously returned a generic message like:

"Backward incompatible schema myTable. Only allow adding new columns"

The exception already carries the full reason (which column was removed, which type changed, etc.) but it was being silently discarded in the catch block. This one-line fix threads e.getMessage() into the response:

"Backward incompatible schema myTable. Reason: Column 'userId' changed type from INT to STRING"

Why it matters: operators debugging failed schema updates had no way to know from the API response what was actually wrong — they had to dig into controller logs. This surfaces the detail directly in the REST response where it belongs.

Change scope: one catch block in PinotSchemaRestletResource, plus a test assertion verifying the message contains the incompatibility detail.

[Akanksha-kedia]

Code Review — PR #18896 (fix/schema-utils-validation-message)

The fix is correct — the exception message from PinotHelixResourceManager (which constructs detailed incompatibility info including missing columns, field-type changes, and suggestions) now flows through to the HTTP 400 response body. Two MAJOR findings on test coverage, several MINOR style issues.

---

C5 — Correctness & Nulls

[MINOR] e.getMessage() null-safety (PinotSchemaRestletResource.java:480)
SchemaBackwardIncompatibleException always passes a non-null message at its only throw site today. However, the constructor has no Objects.requireNonNull guard — a future subclass or test stub could pass null, producing "Reason: null". Consider:

String.format("Backward incompatible schema %s. Reason: %s", schemaName,
    Objects.toString(e.getMessage(), "(no message)"))

[MINOR] Asymmetry between addSchema and updateSchema error format
addSchema (line 439) forwards e.getMessage() with no outer prefix. updateSchema (line 477) now wraps it as "Backward incompatible schema %s. Reason: %s". A user hitting POST /schemas?override=true vs PUT /schemas/{name} gets different message shapes for the same underlying error. Also: the outer prefix "Backward incompatible schema %s." and the inner message both identify the schema by name — the prefix is redundant.

---

C6 — Testing

[MAJOR] No message-content assertion for the addSchema (POST override) backward-incompatibility path
createSchema on line 108 still uses expectValidationException, which only checks the exception type. Since addSchema's catch block also passes e.getMessage(), the detailed-message contract for that path is untested. Add an expectValidationExceptionWithMessage assertion there too.

[MINOR] Asserting on implementation-internal substring "Incompatible field specifications"
This string is constructed in PinotHelixResourceManager. If the wording changes, the test becomes a weaker assertion silently. A more stable anchor is "Reason:" (owned by the REST layer) or the column name being changed.

---

C3 — Naming & API

[MINOR] "Reason:" prefix usage is inconsistent across the file
validateSchemaInternal uses concatenation (". Reason: "); updateSchema's generic Exception catch and the new handler use format strings ("Reason: %s"); addSchema's handler uses no prefix at all. Standardise on one form.

---

C8 — Process & Scope

[MAJOR] Scope gap: only updateSchema was changed, but the test does not validate the pre-existing addSchema message path
addSchema (line 439) was already forwarding e.getMessage() correctly before this PR. The PR correctly fixes only updateSchema. However, there is no test confirming addSchema also surfaces the detail. The PR title says "include validation failure reason" — adding the addSchema test would make coverage complete.

---

C1 — Architecture (advisory)

[MINOR] Detailed incompatibility message is constructed in the resource manager, not in the exception or a value type
The 60-line error-building block in PinotHelixResourceManager.updateSchema() (lines 1626–1685) is user-facing text embedded inside the manager. A cleaner long-term direction: Schema.isBackwardCompatibleWith() returns a structured BackwardCompatibilityResult (list of violations) and the REST layer does the formatting. This PR doesn't make things worse, but flagging for future consideration.

---

Pre-existing issue (not introduced here)

updateSchema returns SuccessResponse(schema.getSchemaName() + " successfully added") (line 472) — should be "successfully updated". Worth a follow-up.