search

substrait-io / substrait

A cross platform way to express data transformation, relational algebra, standardized record expression and plans.
https://substrait.io
Apache License 2.0
1.18k stars 154 forks source link

Inconsistent property name of value argument #639

Closed shanretoo closed 4 months ago

shanretoo commented 4 months ago

The documentation specifies the property name for the type associated with the value argument as Type, yet in the codebase, this property is actually named value. This inconsistency could lead to confusion.

https://github.com/substrait-io/substrait/blob/3dc77aeca820eba70ba141bf3afa5e4c5ba055b9/site/docs/expressions/scalar_functions.md#L27-L32

https://github.com/substrait-io/substrait/blob/3dc77aeca820eba70ba141bf3afa5e4c5ba055b9/text/simple_extensions_schema.yaml#L114-L122

In substrait-java:

  public abstract static class ValueArgument implements Argument {

    @JsonProperty(required = true)
    public abstract ParameterizedType value();
shanretoo commented 4 months ago

Also, the description property is missing in the doc.

westonpace commented 4 months ago

Yes, you are correct. At this point, given we have a lot of YAML files already (and they are using value) I think we should change the site to match the YAML / schema (i.e. change type to value and add description). Are you interested in creating a PR?

shanretoo commented 4 months ago

Sure, I'll draft a pr.

shanretoo commented 4 months ago

Fixed in #640.