raghavyadav01 opened a new pull request, #19698: URL: https://github.com/apache/pinot/pull/19698
An OPEN_STRUCT key's `valueFieldConfigs` only ever took effect while the segment was being written. `OpenStructColumnSplitter` builds the per-key indexes as it writes the child columns, and `OpenStructIndexType.createIndexHandler` returned `IndexHandler.NoOp.INSTANCE`, so nothing revisited them afterwards. The result is that adding a range index to a key and reloading did nothing, while the same change on an ordinary column has always worked: ``` props$cityName + range, reload -> no range_index (OPEN_STRUCT key) id + inverted, reload -> id.inverted_index (ordinary column) ``` ### Cause A materialized child (`props$region`) is a column of the segment but not of the table schema. `FieldIndexConfigsUtil.createIndexConfigsByColName` walks `schema.getColumnNames()`, so a child never gets an entry — and every standard handler derives its column set from that map, leaving the child invisible to all of them. ### Change Give OPEN_STRUCT a real handler. It derives each child's `FieldIndexConfigs` from its `valueFieldConfigs` entry through `FieldIndexConfigsUtil.fromFieldConfig` — which OSS already documents as being *"for synthetic columns (e.g. OPEN_STRUCT materialized children) that exist in no schema"* — then delegates to the ordinary per-index handlers. No index construction is re-implemented; keys are indexed by the same code as every other column, and the two cannot drift. Two details the delegates need: - **Ordering.** The forward handler runs first, then `reloadMetadata()`, then the rest, mirroring `SegmentPreProcessor`'s own scheduling and the comment there requiring it be preserved. A key written raw and later given an inverted index needs the raw-to-dictionary conversion to land first. - **Schema visibility.** The delegates get a schema copy that includes the children. `ForwardIndexHandler` skips a column it cannot find in the schema, which is exactly how a key silently missed that conversion. The table's own schema is untouched. The dense/sparse split is not revisited — which keys are materialized is still decided when the segment is written. ### Testing 3 new tests in `OpenStructPerKeyIndexHandlerTest`; the regression one fails with `NoOp` and passes with the handler (verified by reverting only that line). 169 tests green across every OPEN_STRUCT suite — splitter, mutable, immutable, consuming/sealed parity, multi-value, nested-key, sparse/dense. Also exercised end to end on a segment whose columns are backed by an external Parquet file, where there is no creation-time moment that sees column values at all and per-key indexes were previously impossible. On a 2000-row file with two shredded keys, configuring `inverted` and `range` and reloading produced `props$device_os.inverted_index` and `props$etaInMinutes.range_index`, and the query counters confirm they are used: ``` WHERE props['device_os'] = 'android' -> 500 rows, numEntriesScannedInFilter 0 WHERE props['etaInMinutes'] > 50 -> 300 rows, numEntriesScannedInFilter 0 WHERE props['session'] = 's-5' -> 21 rows, numEntriesScannedInFilter 2000 (sparse key, no per-key index) ``` Removing the config and reloading drops both indexes again. -- 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]
