You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "avshenuk (via GitHub)" <gi...@apache.org> on 2025/07/18 12:51:42 UTC

[PR] Fix unnesting of non-Map value collections. [pinot]

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

   ### Why:
   
   Collections and arrays containing only non-Map values are already being unnested into multiple result rows. But the result leaf values are lost after unnesting.
   Example:
   ```
   {
      "array": ["x", "y"]
   }
   ```
   will be unnested into
   ```
   [
      {
         "array": ["x", "y"]
      },
      {
         "array": ["x", "y"]
      }
   ]
   ```
   
   Which is not very helpful as it loses the original association of the actual unnested value with the corresponding record.
   
   ### What:
   
   Current implementation replaces the unnested field values with the original ones. 
   Which is unnecessary since the original field value is already preserved in the copy of the record regardless (as demonstrated by the tests).
   
   The only reason this replacement would be needed is to explicitly disallow the logic described here: to avoid losing that original array. 
   But that doesn't sound very convincing, as the unnested version in this particular scenario is much more useful, in my opinion.
   
   ### Use cases:
   
   One that comes to mind is:
   - Create multiple copies of a row based on a field like "client ID": you get 2 rows with everything the same except the client ID.
   
   ### Summary:
   
   All in all, the feature seems to be supported already but is just being negated by that replacement logic. If there is any particular reason I've missed, please let me know.


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "avshenuk (via GitHub)" <gi...@apache.org>.
avshenuk commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3138656729

   Yes, this works. Thanks.


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3138064660

   Pushed a commit which fixes the problem. Please take a look and see if it works for you


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "avshenuk (via GitHub)" <gi...@apache.org>.
avshenuk commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3130981019

   > To maintain backward compatible and also fix the issue, we should consider adding back `originalValues` when the key doesn't exist in the unnested record. Could you give it a try and see if it works for the existing test and new added one?
   
   Why would it not exist in the unnested record?


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3090275816

   We should probably add a check to not replace the value if it already exists


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #16387:
URL: https://github.com/apache/pinot/pull/16387


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3133348063

   > > To maintain backward compatible and also fix the issue, we should consider adding back `originalValues` when the key doesn't exist in the unnested record. Could you give it a try and see if it works for the existing test and new added one?
   > 
   > Why would it not exist in the unnested record?
   
   When we unnest a record, the original unnested fields are not included in the record. #13490 added the original fields to the record because the following transformer might use them. We don't want the original unnested fields to override the unnested fields if the key is the same


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3090272311

   This logic was introduced in #13490 to cover a different scenario. cc @xiangfu0


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "avshenuk (via GitHub)" <gi...@apache.org>.
avshenuk commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3092244437

   So from what I can see the logic has changed slightly from that time, and now from looking at `JsonUnnestIngestionFromAvroQueriesTest`, the only difference is that
   ```
   jsonColumn: [
     {"data":{"a":"1","b":"2"},"timestamp":1719390721},
     {"data":{"a":"2","b":"4"},"timestamp":1719390722}
   ]
   ```
   turns into
   ```
   jsonColumn: [
     {"data":{"a":"1","b":"2"},"data.a":"1","timestamp":1719390721,"data.b":"2"},
     {"data":{"a":"2","b":"4"},"data.a":"2","timestamp":1719390722,"data.b":"4"}
   ]
   ```
   So as you can see, the inner maps of such a JSON object are being flattened making the result data different from the original.
   
   So I can see a couple of considerations here:
   1. complex type config always flattens nested maps regardless of "unnest fields" provided. Good or bad, but the user has no control over it, and for a JSON column without arrays it could result in the same issue? 
   **Possible solution**: introduce a new option similar to "unnest fields" but for flattening?
   2. different semantics of complex vs simple items in collections: I can see why one would want to preserve the original value of the collection after unnesting, but there is that inconsistency I mentioned before - each map item will be accessible by the dot separated names after unnesting and yet, not the other non-Map items.
   Moreover, technically, the "root" map is also lost after unnesting (which one might need for whatever reason).
   **Possible solutions**: 
   - Store that root item under a different name?
   - Have an option to control whether to preserve the original value under a different or the same name?
   
   ### Maybe instead we could refactor this transformer into a somewhat cleaner more controllable state?
   1. control what fields to "flatten" - only flattens maps (within collections or not) and only for provided fields
   2. control what fields to "unnest" - only unnests collections (without flattening maps) and only for provided fields
   3. control what fields to keep the original collection value after unnesting for (need to keep the original by default for backward compatibility, but the opposite of that would be the most logical imo).
   4. combining these 3 lets you gracefully decide what end result you want to achieve
   5. allow chaining complex type configs just like transformation configs: e.g. one config flattens, the next one unnests based on the flattened fields and so on
   
   Notes: the original can be preserved temporarily under the new name "<original_name>.$ORIGINAL$" and replaced back to its original name ONLY on the final step of `TransformPipeline`, this way, the unnested object could still be available for further transformations by the user if needed.
   
   ### Examples (config names are subject for discussion):
   Input:
   ```
   [
     {
       jsonField: [
         {"data":{"a":"1","b":"2"},"timestamp":1719390721},
         {"data":{"a":"2","b":"4"},"timestamp":1719390722}
       ]
     }
   ]
   ```
   Config:
   ```
     "fieldsToUnnest": "jsonField"
   ```
   Output:
   ```
   [
     {
       jsonField: {"data":{"a":"1","b":"2"},"timestamp":1719390721},
     },
     {
       jsonField: {"data":{"a":"2","b":"4"},"timestamp":1719390722}
     }
   ]
   ```
   -------------------------
   Config:
   ```
     "fieldsToUnnestAndKeepOriginals": "jsonField",
   ```
   
   Output:
   ```
   [
     {
       jsonField.$ORIGINAL$: [  // becomes `jsonField` in the end
         {"data":{"a":"1","b":"2"},"timestamp":1719390721},
         {"data":{"a":"2","b":"4"},"timestamp":1719390722}
       ],
       jsonField: {"data":{"a":"1","b":"2"},"timestamp":1719390721} // is lost in the end
     },
     {
       jsonField.$ORIGINAL$: [  // becomes `jsonField` in the end
         {"data":{"a":"1","b":"2"},"timestamp":1719390721},
         {"data":{"a":"2","b":"4"},"timestamp":1719390722}
       ],
       jsonField: {"data":{"a":"2","b":"4"},"timestamp":1719390722} // is lost in the end
     }
   ]
   ```
   ----------------------------
   Config 1:
   ```
     "fieldsToFlatten": "jsonField",
     "fieldsToUnnestAndKeepOriginals": "jsonField",
   ```
   Output 1:
   ```
   [
     {
       jsonField.$ORIGINAL$: [  // becomes `jsonField` in the end
         {"data":{"a":"1","b":"2"},"timestamp":1719390721},
         {"data":{"a":"2","b":"4"},"timestamp":1719390722}
       ],
       jsonField: {"data":{"a":"1","b":"2"},"timestamp":1719390721}, // is lost in the end
       jsonField.data: {"a":"1","b":"2"},
       jsonField.timestamp": 1719390721
     },
     {
       jsonField.$ORIGINAL$: [  // becomes `jsonField` in the end
         {"data":{"a":"1","b":"2"},"timestamp":1719390721},
         {"data":{"a":"2","b":"4"},"timestamp":1719390722}
       ],
       jsonField: {"data":{"a":"2","b":"4"},"timestamp":1719390722}, // is lost in the end
       jsonField.data: {"a":"2","b":"4"},
       jsonField.timestamp": 1719390722
     }
   ]
   ```
   Config 2:
   ```
     "fieldsToFlatten": "jsonField.data"
   ```
   Output 2:
   ```
   [
     {
       jsonField.$ORIGINAL$: [  // becomes `jsonField` in the end
         {"data":{"a":"1","b":"2"},"timestamp":1719390721},
         {"data":{"a":"2","b":"4"},"timestamp":1719390722}
       ],
       jsonField: {"data":{"a":"1","b":"2"},"timestamp":1719390721}, // is lost in the end
       jsonField.data: {"a":"1","b":"2"},
       jsonField.data.a: "1",
       jsonField.data.b: "2",
       jsonField.timestamp": 1719390721
     },
     {
       jsonField.$ORIGINAL$: [  // becomes `jsonField` in the end
         {"data":{"a":"1","b":"2"},"timestamp":1719390721},
         {"data":{"a":"2","b":"4"},"timestamp":1719390722}
       ],
       jsonField: {"data":{"a":"2","b":"4"},"timestamp":1719390722}, // is lost in the end
       jsonField.data: {"a":"2","b":"4"},
       jsonField.data.a: "2",
       jsonField.data.b: "4",
       jsonField.timestamp": 1719390722
     }
   ]
   ```


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "avshenuk (via GitHub)" <gi...@apache.org>.
avshenuk commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3133810964

   > > > To maintain backward compatible and also fix the issue, we should consider adding back `originalValues` when the key doesn't exist in the unnested record. Could you give it a try and see if it works for the existing test and new added one?
   > > 
   > > 
   > > Why would it not exist in the unnested record?
   > 
   > When we unnest a record, the original unnested fields are not included in the record. #13490 added the original fields to the record because the following transformer might use them. We don't want the original unnested fields to override the unnested fields if the key is the same
   
   Well. I’ve pushed the changed you asked me to. The test is still failing. Please take a look.


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Fix unnesting of non-Map value collections. [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16387:
URL: https://github.com/apache/pinot/pull/16387#issuecomment-3130134610

   To maintain backward compatible and also fix the issue, we should consider adding back `originalValues` when the key doesn't exist in the unnested record. Could you give it a try and see if it works for the existing test and new added one?


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org