Thanks @karthik-0306 for this contribution, and for calling out the renameCatalogTable() question so explicitly in the PR description. That made it easy to check the one runtime-adjacent spot. I re-derived everything from the source at this head rather than relying on the description, and the change holds up.

What Problem Does This PR Solve?

Pain point: warehouse, database and table were declared as plain .required(OPT) in the Paimon source, sink and catalog factories. An empty or whitespace-only string is a present, non-null value, so it satisfied .required(). A job with warehouse = "" therefore passed ConfigValidator at submission and only failed later, at runtime, with a less actionable error from the catalog or filesystem layer.

Fix approach: migrate these options to .required(OPT, Conditions.notBlank(OPT)), the declarative value-constraint API that is already used by other connectors under umbrella issue #11007.

Example: with warehouse = " ", the job used to be accepted by --check and fail later when the catalog was opened. With this PR, ConfigValidator rejects it up front with an OptionValidationException.

1. Code Change Review

1.1 Core Logic Analysis

Changes in the three factories:

• PaimonSinkFactory.optionRule() and PaimonCatalogFactory.optionRule(): the single .required(WAREHOUSE, DATABASE, TABLE) is split into three .required(OPT, Conditions.notBlank(OPT)) calls.
• PaimonSourceFactory.optionRule(): .required(WAREHOUSE) becomes .required(WAREHOUSE, Conditions.notBlank(WAREHOUSE)). The Conditions import is added there; the sink and catalog factories already imported it.

Before and after, using the sink as the example:

Before: .required(WAREHOUSE, DATABASE, TABLE)
After:  .required(WAREHOUSE, notBlank(WAREHOUSE))
        .required(DATABASE,  notBlank(DATABASE))
        .required(TABLE,     notBlank(TABLE))

Correctness checks I made against the PR head:

• The overloads exist and behave as intended. OptionRule.Builder#required(Option, Condition, Condition...) registers an AbsolutelyRequiredOptions entry and appends the condition to valueConstraints (OptionRule.java:313-324). Conditions.notBlank builds a NOT_BLANK condition (Conditions.java:76-78).
• Splitting one 3-option required(...) into three single-option calls does not change the required-ness semantics. Each option is still absolutely required, and the duplicate check (verifyRequiredOptionDuplicate) is not tripped because each option appears once.
• The option objects used in the conditions are the same ones used in the required declarations in each factory, so the constraints bind to the right keys.
• The source factory deliberately keeps database as optional and table in .exclusive(TABLE, TABLE_LIST). Those are untouched, which is correct: constraining an exclusive or optional option would change the required-ness contract.

Runtime path (normal path, not an edge path): job config -> FactoryUtil / ConfigValidator.validate(optionRule()) -> required-option presence check -> valueConstraints evaluation (NOT_BLANK in ConditionEvaluators.java:79) -> reject or continue to createSource / createSink. This runs on every job submission and --check, so the changed logic is reached by every user, but only blank values behave differently.

The renameCatalogTable() question raised in the description: PaimonSinkFactory.renameCatalogTable() uses StringUtils.isNotEmpty(...) fallbacks for table and namespace. Those fallbacks only matter when the option is absent from the config. The pre-existing rule already declared database and table as absolutely required for the sink, so an omitted option was rejected before this PR as well. The new constraint only affects explicitly blank values, and the fallback never handled a legitimately blank value in a meaningful way (it would have silently substituted the upstream table name for a user typo). I see no behavior regression there.

1.2 Compatibility Impact

Partially incompatible, in the intended and narrow sense: configs that set warehouse, database or table to an empty or whitespace-only string are now rejected at submission instead of failing later at runtime. Non-blank values, including values padded with whitespace, continue to pass, and notBlank does not mutate the configured value. No option was renamed, no default changed, no type changed, and no serialization or checkpoint state is touched. Such configs could not have produced a working job before, so I do not consider this a blocker, and I agree with the author that a separate incompatible-changes.md entry is not needed for what is effectively earlier and clearer failure.

1.3 Performance / Side-Effect Analysis

None. The check is a constant-time string test performed once at config validation, before any connection is made. It avoids a wasted catalog or filesystem attempt for blank input. No threads, locks, retries or resources are involved.

1.4 Error Handling and Logging

No blocking issues found. Failures surface as OptionValidationException from the shared validator, which already formats the option key and the violated constraint, so no extra logging is needed.

Remaining low-severity observations are listed in section 4 (Issues 1 and 2).

Issue 1: Test names and comments do not follow the surrounding test style

• Location: seatunnel-connectors-v2/connector-paimon/src/test/java/org/apache/seatunnel/connectors/seatunnel/paimon/config/PaimonNotBlankValidationTest.java:50-65, 69-71 and similar banner comments
• Problem: Decorative // ---- helpers ---- banner comments, an empty "notBlank tests" banner directly followed by another banner, and snake_case method names (notBlank_sinkEmptyWarehouse). The class Javadoc references the umbrella issue number only.
• Potential risk: Cosmetic only.
• Best improvement: Optionally use camelCase method names and drop the redundant banners.
• Severity: Low
• Raised by another reviewer: No

Issue 2: Negative assertions only check the exception type, not which option failed

• Location: PaimonNotBlankValidationTest.java:85-130 (all assertThrows(OptionValidationException.class, ...) cases)
• Problem: Since each blank case starts from a config where the other required options are valid, the exception can only come from the blank option, so the tests do fail if the notBlank constraint is removed. They would not, however, distinguish "blank rejected by notBlank" from some future unrelated validation failure.
• Potential risk: Very small; a future unrelated rule could mask a regression in these tests.
• Best improvement: Optionally assert that the exception message contains the option key (for example warehouse).
• Severity: Low
• Raised by another reviewer: No

2. Code Quality Assessment

2.1 Coding Standards

The changes are minimal and consistent with how other connectors use Conditions.notBlank under #11007. The ASF license header is present on the new test file. No new core methods or fields were added, so there is no comment gap.

2.2 Test Coverage and Test Stability

PaimonNotBlankValidationTest covers, for the sink warehouse/database/table, the source warehouse, and the catalog warehouse/database/table: a valid config, a missing option, an empty string, a whitespace-only string, and a padded non-blank string. Deleting any of the three notBlank conditions would turn the corresponding empty or whitespace test red, so the tests have real failure power. The padded-value tests also guard against accidental trimming side effects.

Stability: Stable. The tests are pure in-memory validator calls with no containers, no sleeps, no static state and no ordering dependency.

Minor gap, non-blocking: there is no end-to-end case, which I consider acceptable for a declarative rule-layer change and consistent with other #11007 PRs.

2.3 Documentation Updates

docs/en and docs/zh for both the Paimon source and sink were updated consistently ("Must not be blank." and the Chinese equivalent). They match the code: the source doc only mentions warehouse, which is the only option constrained on the source, and the sink doc covers all three.

3. Architectural Soundness

3.1 Elegance of the Solution

Precise fix. It uses the existing declarative validation framework instead of adding imperative checks in the config classes.

3.2 Maintainability

Good. Validation lives in optionRule(), where it is also visible to the generated option documentation and to tooling that reads option rules.

3.3 Extensibility

Fine. Further constraints (for example on catalog_uri when catalog_type is hive) can be added the same way in a follow-up if desired.

3.4 Historical-Version Compatibility

No config key, default, serialization format or checkpoint state changes. Only configs with blank required values change behavior, and those could not have produced a working job.

4. Issue Summary

5. Merge Recommendation

Conclusion: Ready to merge

1. Blockers - must be fixed: none.
2. Recommended fixes - non-blocking: Issues 1 and 2 above, both optional polish.

Overall assessment: this is a focused, low-risk change with matching docs in English and Chinese and tests that fail when the production change is reverted. The renameCatalogTable() concern from the description does not apply, because database and table were already absolutely required for the sink before this PR.

Better alternative: none needed; the declarative route is the right one.

CI and sync status: the apache-side Build check shows action_required with the message "Unable to detect the workflow run for testing the changes in your PR", which means GitHub Actions is not enabled in your fork, so no CI has run on this head. This is a separate blocker for merge, unrelated to the code itself. Could you please enable CI workflows in your forked repository (Actions tab, then enable workflows) and push an empty commit or re-run so the checks start? Reference check run: https://github.com/apache/seatunnel/runs/110123664302 . If you run into any trouble enabling CI, just leave a comment here and we are happy to help. The branch is 29 commits behind dev with no reported conflicts, so no sync is needed beyond that.

Thanks @karthik-0306 for the quick follow-up, for applying both polish suggestions, and for enabling Actions on your fork so CI could run. I re-read the whole PR at this head (9859427) from scratch instead of only looking at the delta since my last approval, and the change still holds up.

Status of my previous items:

• Issue 1 (snake_case test names and banner comments): fixed. The test methods are now camelCase (for example notBlankSinkEmptyWarehouse) and the banner comments are gone from PaimonNotBlankValidationTest.java.
• Issue 2 (negative assertions only checked the exception type): fixed. Every negative case now asserts that the OptionValidationException message contains the failing option key (warehouse, database or table), for example PaimonNotBlankValidationTest.java:83-90. The missing-option cases also check the key, which is a nice extra.

What Problem Does This PR Solve?

Pain point: warehouse, database and table were declared as plain .required(OPT) in the Paimon source, sink and catalog factories. A blank string is a present, non-null value, so it satisfied .required() and only failed later at runtime with a less actionable catalog or filesystem error.

Fix approach: use the declarative value constraint .required(OPT, Conditions.notBlank(OPT)) so blank values are rejected at job submission or --check, in line with umbrella issue #11007.

Example: warehouse = " " used to pass validation and fail when the catalog was opened; now ConfigValidator rejects it up front.

1. Code Change Review

1.1 Core Logic Analysis

The production diff against dev is three small optionRule() edits, unchanged since my last review:

• PaimonSinkFactory and PaimonCatalogFactory: .required(WAREHOUSE, DATABASE, TABLE) is split into three .required(OPT, Conditions.notBlank(OPT)) calls. Required-ness is unchanged and each option appears once, so the duplicate-required check is not tripped.
• PaimonSourceFactory: .required(WAREHOUSE) becomes .required(WAREHOUSE, Conditions.notBlank(WAREHOUSE)), and the Conditions import is added. database stays optional and table stays in .exclusive(TABLE, TABLE_LIST), which is correct because constraining those would change the required-ness contract.

Runtime path (normal path, hit on every job submission and --check): job config -> ConfigValidator.validate(optionRule()) -> required presence check -> valueConstraints evaluation (NOT_BLANK) -> reject, or continue to createSource / createSink. Only blank values behave differently.

The renameCatalogTable() fallback raised in the description only matters when the option is absent. The sink already declared database and table as absolutely required before this PR, so omission was rejected earlier as well. The new rule only affects explicit blank values, so I see no regression for multi-table routing.

The merge-from-dev commit at the head brings in no additional changes to these files; the PR's own diff is still exactly the 8 files listed (3 factories, 4 docs, 1 test).

1.2 Compatibility Impact

Partially incompatible, in the narrow intended sense: configs that set warehouse, database or table to an empty or whitespace-only string are now rejected at submission rather than failing later at runtime. Non-blank values, including padded values, pass unchanged because notBlank does not mutate the stored value. No option name, default, type, serialization or checkpoint state changes. Such configs could not have produced a working job before, so this is not a blocker and a separate incompatible-changes.md entry is not needed.

1.3 Performance / Side-Effect Analysis

None. A constant-time string check runs once at config validation, before any connection is attempted. No threads, locks, retries or resources are involved.

1.4 Error Handling and Logging

No blocking issues found. Failures surface as OptionValidationException from the shared validator, which already names the option and the violated constraint, so no extra logging is needed.

Remaining risks: none that I can attribute to this change. Nothing new to raise this round.

2. Code Quality Assessment

2.1 Coding Standards

Minimal and consistent with other #11007 migrations. License header is present on the new test, naming is now idiomatic, and no new core methods or fields were added, so there is no comment gap.

2.2 Test Coverage and Test Stability

PaimonNotBlankValidationTest covers sink warehouse/database/table, source warehouse, and catalog warehouse/database/table with valid, missing, empty, whitespace-only and padded cases. Removing any single notBlank condition turns the matching empty or whitespace test red, and the message assertions now also pin which option failed. Stability: Stable. The tests are pure in-memory validator calls with no containers, sleeps, static state or ordering dependency. No existing test was weakened; the PR only adds a new test class.

2.3 Documentation Updates

docs/en and docs/zh for Paimon source and sink are updated consistently and match the code (source documents only warehouse, sink documents all three).

3. Architectural Soundness

3.1 Elegance of the Solution

Precise fix, using the existing declarative validation framework.

3.2 Maintainability

Good. The rule lives in optionRule(), visible to option docs and tooling.

3.3 Extensibility

Fine. Further constraints can be added the same way in follow-ups.

3.4 Historical-Version Compatibility

No config key, default, serialization format or checkpoint state changes. Only configs with blank required values change behavior, and those could not have worked.

4. Issue Summary

5. Merge Recommendation

Conclusion: Ready to merge

1. Blockers - must be fixed: none.
2. Recommended fixes - non-blocking: none.

Overall assessment: a focused, low-risk change with matching English and Chinese docs and tests that fail if the production change is reverted. Both of my earlier optional points were addressed exactly as suggested. Better alternative: none needed.

Existing maintainer approvals / change requests: my earlier approval stands; no other reviewer has raised objections.

CI and sync status: in your fork run, all build and test jobs passed (unit-test on ubuntu and windows with JDK 8 and 11, paimon-connector-it, code style, license header, dead links, and the updated-modules jobs). The apache-side Build pointer shows timed_out only because the unrelated "Build website" job in that run was cancelled after about 90 minutes. This is not caused by the PR, which touches only Markdown and Paimon factory rule declarations. If a committer wants a clean pointer, re-running just that website job (or the failed jobs) is enough. The branch is 9 commits behind dev with no conflicts, so no sync is required.

+1