Re: [PR] [core] Add validation to prevent primary key in sequence-group (#7052) [paimon]

["dark2momo (via GitHub)" <gi...@apache.org>]

dark2momo commented on PR #7656:
URL: https://github.com/apache/paimon/pull/7656#issuecomment-4601117074

   @JingsongLi 
   Thanks for the review! Sorry for the oversight — since the broken tests were all **existing** test cases (not newly added ones in this PR), I should have fixed them in the original commit.
   
   To highlight: this PR modifies two **existing** test cases to adapt to the new validation:
   
   - **`PrimaryKeySimpleTableTest#testPartialUpdateWithAgg`**: changed `fields.a.sequence-group` → `fields.b.sequence-group` (since `a` is a primary key)
   - **`PartialUpdateITCase#testProjectPushDownWithLookupChangelogProducer`**: changed `fields.a.sequence-group='j'` → `fields.a.sequence-group='b,c'` (since `j` is a primary key)
   
   The test intent remains the same — only the invalid PK-in-sequence-group config was replaced with a valid non-PK alternative.
   
   Regarding the minor suggestions:
   
   1. **Extract error string to constant** — done. Added `SEQUENCE_GROUP_PK_ERROR` to eliminate the duplicated format string.
   2. **`HashSet` for `primaryKeys.contains()`** — left as `List` as you noted, since PK counts are small at config time.
   3. **Renamed misleading test** — `testMultiPrimaryKeyCannotBeInSequenceGroup` → `testPrimaryKeyCannotBeInSequenceGroupValue` to accurately reflect what it tests.
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org