docs: consistently mark 0 as valid anchor/reference #900
Conversation
benbellick
force-pushed
the
benbellick/consistent-0-anchor
branch
from
c851ebc to
e95927f
Compare
| // to a scalar function in the associated YAML file. Required; avoid | ||
| // using anchor/reference zero. | ||
| // to a scalar function in the associated YAML file. | ||
| // Required; 0 is a valid anchor/reference. |
There was a problem hiding this comment.
This is the only one I am not sure about... I find the language here a bit confusing. Is it saying the two separate things:
- this field is required, and
- you must not use anchor/reference 0
?
Or is it saying "it is required not to use anchor/reference 0"?
I think it is the former. In which case there is nothing special about ScalarFunction.function_reference that prevents it from being 0.
benbellick
force-pushed
the
benbellick/consistent-0-anchor
branch
2 times, most recently
from
0c0da1c to
098e7f9
Compare
benbellick
force-pushed
the
benbellick/consistent-0-anchor
branch
from
098e7f9 to
f32460f
Compare
|
|
||
| // Note: type_variation_reference fields within Type messages reference a | ||
| // type_variation_anchor defined in the plan's extension declarations. The value | ||
| // 0 represents the system-preferred variation and is a valid reference value. |
There was a problem hiding this comment.
Wasn't sure where a good place to put this was considering type_variation_reference shows up a ton of times.
benbellick
requested review from
EpsilonPrime,
cpcloud,
jacques-n,
vbarua,
westonpace and
yongchul
as code owners
|
Given that the proposed documentation changes say "non-negative" would it make sense to always say I do remember the substrait-validator having a check for the 0 anchors reporting a warning. Should probably be also updated at some point in time. |
There are lots of places where the validity of anchor/reference 0 is not explicitly mentioned. This PR adds explicitly clarification in the proto comments and the site docs to clarify that anchor/reference 0 is valid.
Closes #899