tarun11Mavani commented on code in PR #19608:
URL: https://github.com/apache/pinot/pull/19608#discussion_r4119358443
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java:
##########
@@ -106,7 +115,33 @@ public DataSource getDataSource(String key) {
return new NullDataSource(getValueFieldSpec(key),
getDataSourceMetadata().getNumDocs());
}
return _sparseKeyDataSourceCache.computeIfAbsent(key,
- k -> new SparseKeyDataSource(getValueFieldSpec(k), _sparseBlobReader));
+ k -> new SparseKeyDataSource(getValueFieldSpec(k), _sparseBlobReader,
maxNumValues(k)));
+ }
+
+ /// Field spec for a key's values, with an undeclared sparse key's shape
taken from the segment's sparse
+ /// multi-value manifest. Which tier a key lands on is a tuning decision, so
it must not decide the key's
+ /// shape: without this, the same rows would report `STRING[]` on a segment
that materialized the key and a
+ /// scalar `STRING` holding `["a","b"]` on one that put it in the blob, and
a query fanning out over both
+ /// would see two shapes for one column.
+ @Override
+ public FieldSpec getValueFieldSpec(String key) {
+ FieldSpec childFieldSpec = _fieldSpec.getChildFieldSpec(key);
+ if (childFieldSpec != null) {
+ return childFieldSpec;
+ }
+ boolean singleValue = _sparseMultiValueKeys == null ||
!_sparseMultiValueKeys.containsKey(key);
+ return new DimensionFieldSpec(key, FieldSpec.DataType.STRING, singleValue);
Review Comment:
I think follow up should be fine as it's not directly introduced in this PR.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/openstruct/OpenStructColumnSplitter.java:
##########
@@ -196,62 +212,125 @@ public Set<String> classify() {
private void addMap(@Nullable Map<String, Object> map) {
if (map != null && !map.isEmpty()) {
- for (Map.Entry<String, Object> entry : map.entrySet()) {
- String key = entry.getKey();
- Object rawValue = entry.getValue();
- if (rawValue == null) {
- continue;
- }
- if (_config.isIgnoredKey(key)) {
- _ignoredKeyDropCount++;
- continue;
- }
- FieldSpec keySpec = _childFieldSpecs.get(key);
- DataType valueType;
- if (keySpec != null) {
- valueType = keySpec.getDataType();
+ OpenStructKeyFlattener.flatten(map, _maxNestedKeyDepth, this::addEntry);
+ }
+ _numDocs++;
+ }
+
+ /// Accumulates one flat key of the current document. `container` marks a
key whose value is a nested object
+ /// rendered as JSON text; see [#classify()] for why those are held out of
automatic dense selection.
+ private void addEntry(String key, @Nullable Object rawValue, boolean
container) {
+ if (rawValue == null) {
+ return;
+ }
+ if (_config.isIgnoredKey(key)) {
+ _ignoredKeyDropCount++;
+ return;
+ }
+ if (container) {
+ _containerKeys.add(key);
+ }
+ FieldSpec keySpec = _childFieldSpecs.get(key);
+ // Shape is decided by the first value the key presents and then sticks,
exactly as its type does. A
+ // collection arriving on a key whose shape is already scalar is handled
as any other value it cannot
+ // represent -- stringified on a STRING key, a coercion failure on a typed
one -- rather than reshaping a
+ // column other documents already wrote to.
+ Object[] elements = OpenStructTypeInference.asMultiValue(rawValue);
Review Comment:
Since it's first sighting rather than each row, I think it should be fine.
--
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]
