xiangfu0 commented on PR #18544: URL: https://github.com/apache/pinot/pull/18544#issuecomment-4557595936
Pushed `93b02a98d5` as a follow-up review/polish commit. Summary: **Simplifications (~410 net lines removed)** - Delete `MaterializedViewSchemaInferer` interface + factory; collapse the single impl into a final class with a static `infer()`. - Inline `MaterializedViewPropertyExtractor.isMaterializedView()` to direct `TableConfig.isMaterializedView()` calls. - Collapse 3 identical `IF NOT EXISTS` race-recovery checks into `mvIfNotExistsNoOpResponse(...)`. - Replace `MaterializedViewPropertyRouter.canonicalTaskKnobKeys()` `Set` lookup with a single `canonicalKnobName(lowerCasedKnob)` helper. **Bug fixes** - Canonicalize `task.MaterializedViewTask.*` prefix-form keys in the router so bare and prefixed forms land under the same on-wire key. - `executeDdl` no longer catches `RuntimeException` as 400 — programmer errors now surface as 500 so monitoring fires. - Wrap `IllegalStateException` from `RequestUtils.extractAliasOrIdentifierName` as `DdlCompilationException` so unaliased aggregations (e.g. `SELECT SUM(x) FROM …`) surface as 400 with guidance, not 500. - `IF NOT EXISTS` race-catch paths verify `isMaterializedView()` — a concurrent plain OFFLINE table create no longer fools MV DDL into returning 200. - `TaskCleanupRestorer` restores task schedules when `deleteTable` rejects DROP (e.g. blocked by dependent MV); the misleading "schedules need manual restoration" hint is suppressed after a successful restore. - `MaterializedViewConsistencyManager.rebuildReverseIndex` skips orphan definition znodes (TableConfig missing or non-MV) with a WARN log; prevents ghost MV resurrection after a partial DROP. - `MaterializedViewAnalyzer` validates `stalenessThresholdMs` at CREATE time (rejects malformed/negative; explicit `0` for "no SLO" is accepted). - HTTP-header database threaded into `DdlCompileContext` so the MV inferer resolves `FROM` against the operator's intended DB instead of the cluster default. **Tests added** - Prefix-form case canonicalization, unaliased aggregation, orphan-znode skip, `stalenessThresholdMs` validation (malformed/negative/zero), `IF NOT EXISTS` race-catch against a plain OFFLINE table. All affected modules pass `mvn test` + `spotless:apply checkstyle:check license:check` locally. Waiting on full CI. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
