SEPURI-SAI-KRISHNA commented on PR #57832: URL: https://github.com/apache/spark/pull/57832#issuecomment-5604765026
@uros-b checked, and the same pattern is there in both. I also swept the rest of the file rather than just those two, so we can close the question instead of leaving it open. **concat and flatten** `Concat.genCodeForNumberOfElements` and `Flatten.genCodeForNumberOfElements` both accumulate the element count into a `long` and hand it straight to `CodeGenerator.createArrayData` with no `MAX_ROUNDED_ARRAY_LENGTH` check, exactly like `array_repeat` before this PR. Their interpreted paths do check, and raise `COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION`. Codegen instead reaches `ArrayData.allocateArrayData`, whose own bound check raises the internal `_LEGACY_ERROR_TEMP_2176`. Same shape of divergence, same fix. **Everything else that touches the limit** | expression | interpreted | codegen | | | --- | --- | --- | --- | | `array_repeat` | checks | no check | fixed here | | `array_insert` | checks | checks | fixed here | | `concat` | checks | **no check** | gap | | `flatten` | checks | **no check** | gap | | `sequence` | checks | shared | not a gap, see below | | `array_distinct`, `array_union` | checks | checks | consistent | | `array_intersect`, `array_except` | none | checks | result is a subset of an input, so the limit is unreachable and the codegen check is defensive | | `map_zip_with` | checks | none | `CodegenFallback`, so there is no codegen path | `sequence` is worth calling out because it looks like a third gap: it checks in `sequenceLength` and has no `MAX_ROUNDED_ARRAY_LENGTH` in its generated code. It is not one. Its codegen calls `Sequence.sequenceLength` directly, so both paths go through the same check. Anyone grepping for the pattern will land on it, so it seemed worth recording why it is fine. I looked outside this file too. The other users of the constant are infrastructure rather than expression pairs: `ByteArrayMethods`, `BufferHolder`, `WritableColumnVector`, `ArrayData`, `UTF8StringBuilder`, `StringConcat` and the core buffers. **How much this matters** `array_repeat` reaches the limit cheaply, since `count` is just an argument. `concat` and `flatten` need the inputs to hold that many elements for real, so the executor runs out of memory long before the check is reached. The divergence looks latent rather than something a user hits today, which fits your read that nothing needs doing here now. I also checked the branch above that check, since `allocateArrayData` narrows with `numElements.toInt` when it picks `UnsafeArrayData`. That branch is not reachable with an oversized count: for any element size of at least one byte, `shouldUseGenericArrayData` is already true by the time the length passes `Int.MaxValue`, so the generic branch and its bound check always win. No wrong-result risk, only the error condition. Happy to open the tickets. Would you like one each for `concat` and `flatten`, or a single ticket covering both, given the fix is the same shape in each? -- 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]
