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.
There was a problem hiding this comment.
I can confirm that the intent of this is:
- It is required that this field be set.
- 0 is a valid anchor / reference
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
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. |
|
I can provide some guidance on why the non-zero language exists for references. Effectively, if a The stance of the project has generally been What do you think about including the explicit guidance to avoid zero values? |
vbarua
left a comment
vbarua
left a comment
There was a problem hiding this comment.
Thanks for updating this. Left on minor comment, but otherwise looks good to me.
|
Should we also update tutorials for URN anchors to start from 0? Edit: I just reread this to understand:
I guess the idea is that anchors are unique, unordered values and they're all valid as long as they're unique (within a plan) at any given time? |
…ference comments - Update type_variation_anchor comment to clarify 0 is reserved for system-preferred variation - Reformat type.proto header comment for better readability - Remove redundant anchor/reference comments from all *_reference fields since guidance on anchor definitions is sufficient
5 participants
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