havefunatcode commented on issue #18010: URL: https://github.com/apache/druid/issues/18010#issuecomment-6483925270
Thanks, that makes sense. I'll go with that in this PR and leave SUM/AVG for a follow-up. Roughly: - APPROX_QUANTILE, APPROX_QUANTILE_FIXED_BUCKETS and APPROX_QUANTILE_DS (plus DS_QUANTILES_SKETCH and the TDIGEST ones, which have the same `ANY` first operand) declare it as NUMERIC and list the complex types they accept. - `DefaultOperandTypeChecker` gets opt-in builder options for allowed complex types and for coercing an operand, so the other functions using it don't change. Non-numeric string literals are rejected before coercion, otherwise `'m1'` would just turn into a null. One more thing: the checker rejects a NULL literal for a required operand, so `APPROX_QUANTILE(NULL, 0.5)` would start failing with "Illegal use of 'NULL'" once the first operand isn't `ANY`. I think that's fine. Other functions using the checker already behave that way (`BITWISE_AND(NULL, 1)`, `SAFE_DIVIDE(NULL, 1)`, `TRUNCATE(NULL, 0)`), and I couldn't find a quantile test that relies on NULL being accepted. Keeping it would need a "required but nullable" option, which doesn't really fit the type check, so I'd rather not add one. That also means leaving the Spectator functions on `OperandTypes.ANY` for now, since `SpectatorHistogramSqlAggregatorTest` expects `SPECTATOR_COUNT(null)` and `SPECTATOR_PERCENTILE(null, 99.9)` to return null. Does that sound OK to 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: [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]
