andygrove commented on PR #2644: URL: https://github.com/apache/datafusion-comet/pull/2644#issuecomment-3503133494
> > > > This was done so that we could have different outputs for different use cases. The less verbose use case was intended to allow an analytic system to query fallback reasons, so the output was simple and easily parsable in Spark SQL. The more verbose use case was intended for human consumption via the Spark UI. (Comet extended info can be displayed in the Spark UI's SQL tab, since Spark 4.0) > > > > > > > > > @parthchandra Are you ok with the change to remove the option, or do you think we need to keep this functionality? > > > > > > I feel we must have the verbose output available at all times so that users can get that in the Spark UI and explain plan. Whether it is behind a config option or not is up to us. The option to get an output that is easier to parse in an automated system is up to us to decide collectively. Any analysis of the verbose output will involve custom parsing (regexps in the simplest cases) that will be broken every time someone adds a new piece of information. I would prefer we keep this option too, but you can change my mind :) Perhaps we can add a unit test to verify that the output can be parsed can catch breaking changes to the output. > > ok, I largely reverted the changes and changes the config from an `enabled` boolean to a string that allows a specific format to be chosen, defaulting to verbose. This is more flexible for adding more formats in the future. @parthchandra Could you take another look when you get a chance -- 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]
