Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start draft of KDL Data Model #228

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

nichtich
Copy link

@nichtich nichtich commented Oct 7, 2021

Thanks for discussion (#225). In addition to further changes to the specification of KDL Data model, the document should be referenced from other documents in this repository.

@zkat zkat added the breaking This can only be done for the next major version of KDL label Oct 7, 2021
MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
nichtich and others added 3 commits October 8, 2021 09:14
* Include suggestions from review
* Add missing tags on values
* Add sections on data binding and limited support of the data model
Comment on lines +89 to +104
### Limitations to the data model

Implementations may choose to limit the set of processable KDL documents for
technical reasons. Such limitations must be stated clearly to indicate a useful
but incomplete support of KDL data model. Reasonable limitations include:

* Precision of numbers

* Types of elements that can have tags (e.g. disallow tags for boolean values
and `null`)

* Unicode Normalization (e.g. collapse properties into one when their names are
equivalent after normalization)

Implementations must document how limitations to KDL model are applied when KDL
document are read (e.g. give warnings and ignore unsupported elements).
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot tags on values in the data model. Things like unlimited precision numbers and type annotations of a null may be needless for many implementations so they may chose to not support the full data model. Nevertheless these edge cases are part of the model and may have use cases (e.g. (unknown)null vs. (not-applicable)null).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that numbers and strings don't have a "tag" concept natively in any language I know of, and thus have to be handled with kdl-specific data types anyway, requiring tags to be supported on bools/nulls as well doesn't seem to be any additional burden.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this section is non-normative anyway and does not need to be part of the data model document. Maybe the section should better explain difference between syntax, parsing, and data model (again?). Treatment of tagged numbers and may need further description anyway: KDL Schema mentions format without reference to tag/type.

@tabatkins
Copy link
Contributor

+1, these changes are nice. I can update the tests to mandate empty tags not be preserved in serialization once this is merged.

@zkat zkat changed the base branch from main to kdl-v2 October 12, 2021 01:16
@zkat
Copy link
Member

zkat commented Oct 12, 2021

moving this to kdl-v2

MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
MODEL.md Show resolved Hide resolved
Copy link
Contributor

@tabatkins tabatkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per discussion in #233, Kat would prefer empty tag and missing tag stay distinct, so let's capture that.

MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
MODEL.md Outdated Show resolved Hide resolved
@nichtich
Copy link
Author

moving this to kdl-v2

I don't think the data model breaks the KDL spec because KDL spec does not define how to internally store the result of parsing. A model is more important for KQL and KDL Schema. Nevertheless it would help to state that a KDL parser MUST preserve at least the information covered by the data model.

@tabatkins
Copy link
Contributor

Agreed, this should just be capturing the assumptions inherent in the test-suite, and so is suitable for v1.

@zkat zkat changed the base branch from kdl-v2 to main October 13, 2021 05:45
@zkat zkat removed the breaking This can only be done for the next major version of KDL label Oct 13, 2021
@zkat
Copy link
Member

zkat commented Oct 13, 2021

Noted. Moved back :)

@zkat
Copy link
Member

zkat commented Aug 28, 2022

Shall we roll this up as part of KDL 2.0?

@zkat zkat mentioned this pull request Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants