Jackie-Jiang commented on PR #19087: URL: https://github.com/apache/pinot/pull/19087#issuecomment-5656452685
After considering the remaining correctness issues and future precision support, I recommend closing this PR and relying on `sourceFieldConfigs` for explicit conversion of non-schema source columns. The fundamental problem is that an aggregation function does not establish a source's actual type, accepted representation, or a conversion that is safe for every consumer. The compatibility veto fixes the direct SUM/HLL example, but shared-field conversion still affects indirect consumers through transforms, and even numeric consumers can have different shape and null-handling requirements. This also constrains future improvements to SUM/MIN/MAX for LONG inputs. Automatically converting `9007199254740993L` to DOUBLE loses precision before aggregation begins. A future aggregator with exact LONG support could no longer recover the original value. We should avoid baking today's double-based implementation into the ingestion pipeline. `sourceFieldConfigs` provides an explicit input-type contract: users can choose LONG, BIG_DECIMAL, DOUBLE, etc., and use separate columns when consumers need different representations. We should preserve the existing explicit-conversion behavior, including intentional conversions for HLL/sketch inputs. Documentation and examples for this mechanism would be a useful follow-up. Preventing partial-row mutation remains a separate concern. Explicit typing cannot catch every aggregation overflow or invalid value produced by later transforms; that needs validation before segment mutation or atomic row updates. I'm sorry I did not catch this fundamental design issue earlier, and for steering this through several iterations before reaching this conclusion. Thank you for working through the feedback and adding the regression coverage. -- 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]
