raghavyadav01 opened a new pull request, #19581:
URL: https://github.com/apache/pinot/pull/19581
`BaseDefaultColumnHandler` carries three TODOs that limit derived columns
built at segment load. This lifts all three.
### Chained derived columns
`updateDefaultColumns` iterated an unordered map, and arguments were
resolved against the segment metadata loaded *before* the run, so a column
created during the run was invisible. A config like
```json
"transformConfigs": [
{"columnName": "event_hour", "transformFunction": "dateTrunc('HOUR',
event_ts, 'MILLISECONDS')"},
{"columnName": "event_day", "transformFunction": "dateTrunc('DAY',
event_hour, 'MILLISECONDS')"}
]
```
silently produced default values for `event_day`. Columns are now visited in
dependency order (topological over the ADD actions) and the metadata written
for each created column is recorded and consulted before the segment metadata.
The same bug affected a derived column reading a **plain default column**
added in the same reload, which is also fixed and covered by a test.
A dependency cycle cannot be satisfied in any order, so those columns are
emitted last and take the default-value path, with the cycle logged rather than
failing the segment load.
### Derived column whose source has the forward index disabled
Previously an unconditional `UnsupportedOperationException`. The forward
index is now regenerated from the dictionary and inverted index via
`InvertedIndexAndDictionaryBasedForwardIndexCreator` for the duration of the
run and dropped afterwards, mirroring the temporary-forward-index pattern
`BaseIndexHandler` already uses. When regeneration is not possible (no
dictionary or inverted index) the column falls back to its default value, or
still fails when `errorOnColumnBuildFailure` is set, preserving the previous
strictness.
### Forward-index-disabled derived column
Previously skipped with a warning. It is now built with a forward index and
a dictionary; `ForwardIndexHandler` runs immediately after, queues
`DISABLE_FORWARD_INDEX`, and deletes the forward index in its post-update
cleanup once the inverted index has been built from it — the same sequence
non-derived columns already go through.
### Supporting changes
- `V3DefaultColumnHandler` decided whether to transfer a forward or an
inverted index into the v3 file from the *config*. A forward-index-disabled MV
**default** column writes an inverted index but a **derived** one writes a
forward index, so the decision now follows the file that exists on disk.
- `DefaultColumnHandlerFactory` gains a `SegmentDirectory` overload, needed
by the regenerator. The existing 4-arg signature is kept and delegates with
`null`.
### Performance
For a table with **no** derived columns the path is unchanged: no expression
is parsed, and `orderByDependencies` returns the original action map by
identity so iteration order is identical. With derived columns, the transform
configs are indexed by column and parsed expressions cached, so each expression
is parsed once per segment load instead of once for ordering and again for
deriving. The metadata read-back after creating a column is restricted to
columns a transform function actually reads, so a schema evolution adding many
plain default columns does not re-parse metadata nothing will consult.
### Testing
New tests in `SegmentPreProcessorTest`, on both segment versions unless
noted:
- `testChainedDerivedColumns` — transform configs listed dependent-first, so
it fails if ordering is not applied
- `testDerivedColumnReadingDefaultColumnAddedInSameReload`
- `testDerivedColumnCycleFallsBackToDefaultValue`
- `testDerivedColumnFromForwardIndexDisabledSource` (v3 only — a v1 reload
cannot remove a forward index)
- `testForwardIndexDisabledDerivedColumn` (v3 only, same reason)
Existing suites pass unchanged: `SegmentPreProcessorTest` (65),
`ForwardIndexHandlerTest` (42), `LoaderTest`, `DefaultColumnHandlerTest`,
`DefaultColumnStatisticsTest`, `ColumnMetadataTest`,
`SegmentLocalFSDirectoryTest`, `IndexLoadingConfigTest`, `LoaderUtilsTest`,
`ForwardIndexHandlerContextTest`, `ForwardIndexHandlerCompressionStatsTest` —
177 total.
Also exercised end to end on a 5M-row table with day/hour/minute `dateTrunc`
derived columns, a chained derived column, and a forward-index-disabled derived
column, verifying dictionary, forward, inverted, range, bloom, FST, text and
star-tree indexes all build and serve queries on derived columns.
--
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]
