The config values aren't checked
> set datafusion.explain.format = indnet;
0 row(s) fetched.
Elapsed 0.002 seconds.
> set datafusion.explain.format = indent;
0 row(s) fetched.
Elapsed 0.002 seconds.
The indnet doesn't belong to format, obviously, it's a wrong value. But I didn't get any hints for it.
It'll be great UX if we can give some hints for the wrong config values.
@Weijun-H do you think we can make the follow-up changes as good first issues?
Yes, I think it’s a good first issue because it need wrap up the current logic and is a good way for new contributors to become familiar with DataFusion.
take
Hey @xudong963 @Weijun-H
I was not able to reproduce this issue with the latest build:
What do you think if some more improvements are required?
P.S. I did some digging and found that suggest_valid_function is already available. So a potential improvement could be something like "did you mean" suggestions.
To close this ticket I think we'd need to apply this fix to all other config values which expect a certain set of values (i.e. enums) but still parse as string. For example:
https://github.com/apache/datafusion/blob/b9517a1a08c573af2ee0e8814413eb5d96b2e5bd/datafusion/common/src/config.rs#L283-L291
This option specifies a list of valid values but it accepts arbitrary strings.
@etolbakov Maybe you missed it, but there was already a PR submitted and merged above that resolved this for the explain format configuration, only. At the end of that PR, there was this comment (https://github.com/apache/datafusion/pull/17549#pullrequestreview-3221955466), asking to implement it for others. That PR serves as a good example of how to implement it for other config values.
Thanks @Jefffrey @petern48 Now it makes sense!
Hey @xudong963 @Weijun-H
I was not able to reproduce this issue with the latest build:
What do you think if some more improvements are required?
P.S. I did some digging and found that suggest_valid_function is already available. So a potential improvement could be something like "did you mean" suggestions.
I think 'did you mean' is a good idea too.
The timezone config option would be another to add validation to.
What do you think if some more improvements are required?