xiangfu0 opened a new pull request, #19687:
URL: https://github.com/apache/pinot/pull/19687

   ## Context
   
   Follow-up to #19308, which started rejecting a per-key `codecSpec` on 
OPEN_STRUCT keys. **Stacked on #19308**: this PR's base is its branch, so merge 
#19308 first; this PR will then be retargeted to `master` and rebased.
   
   ## Problem
   
   An OPEN_STRUCT column materializes dense keys as child columns (e.g. 
`payload$clicks`), each configured through a per-key `FieldConfig` in 
`valueFieldConfigs` or `defaultValueFieldConfig`. Only part of that 
`FieldConfig` is honored; the rest is silently dropped:
   
   | Per-key setting | At segment creation and on reload |
   |---|---|
   | `encodingType` | Honored. DICTIONARY builds a dictionary-encoded key; RAW 
builds a raw LZ4 key, unless an enabled index that requires a dictionary (such 
as inverted) makes it dictionary-encoded |
   | `indexes.inverted`, `indexes.range`, `indexes.bloom` | Honored. A per-key 
config without `inverted` builds no inverted index |
   | `indexes.forward` | Parsed, then replaced: `OpenStructColumnSplitter` 
always builds a dictionary-encoded or LZ4 raw forward index |
   | `indexes.dictionary` | Never read: `FieldIndexConfigsUtil.fromFieldConfig` 
derives the dictionary from `encodingType` |
   | `compressionCodec`, `indexTypes`, `timestampConfig`, `properties`, 
`tierOverwrites` | Never read |
   
   Traced through `OpenStructColumnSplitter.writeDenseKeyColumn` / 
`resolveUseDictionary` (segment creation) and 
`IndexLoadingConfig.withOpenStructChildConfigs` → 
`FieldIndexConfigsUtil.fromFieldConfig` (reload, where the inverted, range and 
bloom handlers act on child columns and `ForwardIndexHandler` skips them).
   
   ## Changes
   
   - `OpenStructIndexType` table-config validation rejects each ignored 
setting, naming the OPEN_STRUCT column and the key or 
`defaultValueFieldConfig`, for example: `OPEN_STRUCT column 'payload': 
indexes.forward.targetDocsPerChunk is not supported for key 'clicks'; 
materialized keys always use a dictionary-encoded or LZ4 raw forward index`.
     - `indexes.forward` may only restate the key's `encodingType` or keep 
`disabled: false`. `indexes.dictionary` may only carry a `disabled` value 
consistent with `encodingType`; an empty `{}` on a RAW key enables the 
dictionary at table level, so it is rejected.
     - Empty `properties` / `tierOverwrites` and JSON nulls count as unset. The 
`codecSpec` message from #19308 is unchanged.
   - The `OpenStructIndexConfig` class Javadoc lists the honored settings, 
correcting the per-key dictionary claim added in #19308.
   - No new build-time guard in the splitter: validation is the enforcement 
point, and failing segment builds over an ignored tuning setting would stall 
ingestion for tables saved with validation skipped. The `codecSpec` splitter 
guard from #19308 stays.
   
   ## Compatibility
   
   Validation tightening only. OPEN_STRUCT has not been released, and no 
quickstart, test resource, doc or in-repo test config sets a newly rejected 
setting. A table that already carries one keeps loading and ingesting; its next 
table-config update fails validation until the setting is removed.
   
   ## Testing
   
   - `OpenStructIndexTypeTest` (14 → 36 tests): 19 rejected-setting cases, 
honored and restated settings accepted, `defaultValueFieldConfig` naming, and 
an end-to-end `TableConfigUtils.validate` JSON round trip.
   - In-reactor with JDK 25: `*OpenStruct*Test`, `TableConfigUtilsTest` and 
`SegmentMetadataImplTest` across `pinot-spi` and `pinot-segment-local`, 302 
tests, 0 failures.
   - Spotless, license and checkstyle clean on `pinot-spi` and 
`pinot-segment-local`.
   


-- 
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]

Reply via email to