discord9 commented on PR #22837: URL: https://github.com/apache/datafusion/pull/22837#issuecomment-4727035244
> Thank you @discord9 -- I think this PR fixes one bug (but there is at least one more -- with decimals) > > After some more research I agree a more general allow list would be a better design (which is what you are trying in #21908. I am sorry, but it was too complicated to grok what was going on without additional context) > > I recommend we either: > > 1. Merge this PR, file a ticket to track the decimal bug, and work on the general design in a follow on PR > > 2. Try rework this PR to have a more general allow list structure > > > I personally suggest we do 1 to optimize review speed, but let me know what you think. If you agree, perhaps you'll be able to work on the more general allow list PR (and we can work on making it smaller / easier to review) > > Let me know your thoughts > > Thank you for working on this We can merge this PR first and then discuss this one https://github.com/apache/datafusion/pull/22906 maybe? This is in a way a more general allow list, and also a more powerful one which allow some cases which normal simple allow list wouldn't allow(Like change literal value or transform expr to other formals, like `cast(a as t) = lit` to `a >=lit_lowbound AND a<lit_highbound`, it's basically the same `preimage` way you did with `udf_preimage`, but now with `cast`! Anyway let me polish this pr better before merge -- 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]
