feat: add support for specifying DimensionSchema to catalog clustered segment spec - #19770
Conversation
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 6 of 6 changed files.
This is an automated review by Codex GPT-5.6-Sol
| { | ||
| final Map<String, DimensionSchema> customSchemas = new HashMap<>(); | ||
| for (DimensionSchema schema : columnSchemas) { | ||
| if (customSchemas.put(schema.getName(), schema) != null) { |
There was a problem hiding this comment.
[P2] Reject null column-schema entries
Jackson permits null list elements, so a payload such as "columnSchemas":[null] reaches catalog validation and throws an NPE at schema.getName() instead of returning an InvalidInput response. Explicitly reject null entries before indexing the schemas.
FrankChen021
left a comment
There was a problem hiding this comment.
I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.
Since the prior review, null columnSchemas entries are explicitly rejected with InvalidInput and covered by a regression test. The prior null-dereference issue is addressed.
Reviewed 6 of 6 changed files.
This is an automated review by Codex GPT-5.6-Sol
Description
Follow-up to #19711, this PR allows optionally specifying the
DimensionSchemaof any columns defined in the catalog clustered segment metadata. This is used to allow customization of how the column is physically stored instead of using the the default for the type defined in the logical schema. The type must match the logical type.