tarun11Mavani commented on code in PR #18710:
URL: https://github.com/apache/pinot/pull/18710#discussion_r3386293451
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/OpenStructNaming.java:
##########
@@ -36,4 +36,41 @@ public static String materializedColumnName(String
openStructColumn, String key)
public static String sparseColumnName(String openStructColumn) {
return openStructColumn + SEPARATOR + SPARSE_SUFFIX;
}
+
+ /// Returns true if the given column name is a materialized OPEN_STRUCT
child column
+ /// (dense materialized key or the sparse JSON column).
+ public static boolean isMaterializedOpenStructColumn(String columnName) {
+ return columnName.indexOf(SEPARATOR.charAt(0)) > 0;
+ }
+
+ /// Returns true if the given column name is the sparse JSON column for some
+ /// OPEN_STRUCT parent.
+ public static boolean isSparseColumn(String columnName) {
+ int sep = columnName.indexOf(SEPARATOR.charAt(0));
Review Comment:
In this PR, the only caller is `TableConfigUtils` — it validates that no
user-defined column contains `$` when the schema has an OPEN_STRUCT field. This
runs at table creation/update time before any segment exists, so segment
metadata isn't available. Name-based detection is the only option here.
In PR 2b (storage layer), the callers are:
- `OpenStructColumnSplitter` — the name *producer* at seal time
- `ImmutableSegmentImpl` — already uses
`ColumnMetadataImpl.isMaterializedChild()` as the primary gate
(metadata-based), then uses naming helpers only to distinguish dense vs sparse
and extract the key name
- `IndexLoadingConfig` and `ImmutableSegmentLoader` — currently use naming
as the first filter; I'll tighten these to use metadata as the primary gate in
PR 2b
So the approach is dual: metadata (`PARENT_COLUMN`, `isMaterializedChild()`)
is the authoritative source at segment load time; naming is a convenience for
pre-segment contexts where metadata doesn't exist yet.
I've updated the class-level and method-level Javadoc on `OpenStructNaming`
to make this explicit — the name-based helpers are documented as loose
heuristics with possible false positives, and callers with segment metadata are
directed to prefer `ColumnMetadataImpl.isMaterializedChild()` instead.
--
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]
