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

Added jsonSchema constraint to object and array fields #32

Merged
merged 5 commits into from
Feb 21, 2024

Conversation

roll
Copy link
Member

@roll roll commented Feb 6, 2024


Rationale

Please see the attached issue.

Note

I think the initial wording didn't mention that object needs to be an object, not just JSON by mistake. It's been fixed here.

@peterdesmet
Copy link
Member

Do I understand correctly there is currently no real use case for this: frictionlessdata/specs#640 (comment)?

@roll
Copy link
Member Author

roll commented Feb 20, 2024

@peterdesmet
I think it's hard to say because the object type was once requested, meaning that people would like to use fields for complex data structures, and JSON Schema validation is just a standard feature for sanitizing them. Probably, we will be kind pro-active here but taking into account that for implementations it's basically no-code addition I think it's a good feature candidate

Co-authored-by: Peter Desmet <peter.desmet.work@gmail.com>
Copy link

cloudflare-pages bot commented Feb 20, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2edda59
Status: ✅  Deploy successful!
Preview URL: https://ccb793c3.datapackage.pages.dev
Branch Preview URL: https://640-json-schema-constraint.datapackage.pages.dev

View logs

@roll roll added the candidate label Feb 20, 2024
@roll
Copy link
Member Author

roll commented Feb 20, 2024

@ezwelty
@PietrH
Can you please take a look?

@ezwelty
Copy link

ezwelty commented Feb 20, 2024

Why must it be a JSON object ({...}) rather than any JSON? After all, JSON Schema can describe any JSON (object, array, string, number, boolean, or null), and if we restrict to JSON object, then there is no way – for example – to express an array of objects or a column that can contain either null or some object.

So I vote in favor of the jsonSchema property while continuing to allow any JSON and perhaps plan to eventually rename object (and array) to json.

@roll
Copy link
Member Author

roll commented Feb 20, 2024

@ezwelty
In my opinion, it's quite OK that basically, here are two json types:

  • object -- json type requiring a top-level object
  • array -- json type requiring a top-level array

I'm not sure if having one arbitrary json type will benefit data producers/consumers as communicating any other json type like string/number doesn't really make sense and object/array distinction simplifies data modeling because usually programming language have clear distinction between sequence and mapping types.

But the bottom line that both are just aliases to json with corresponding top-level restrictions

@roll
Copy link
Member Author

roll commented Feb 20, 2024

If personally, I were modeling type system from scratch I might actually use hierarchical types like:

  • json
  • json/array
  • json/object
  • json/object/geojson
  • etc

But we have what we have an I guess it just works fine

@ezwelty
Copy link

ezwelty commented Feb 20, 2024

In my opinion, it's quite OK that basically, here are two json types:

  • object -- json type requiring a top-level object
  • array -- json type requiring a top-level array

Fine. In that case, this new jsonSchema property should also be available to the array type.

But wouldn't jsonSchema better belong in field.constraints and be available to types integer, number, boolean, string, object, array?

@peterdesmet
Copy link
Member

But wouldn't jsonSchema better belong in field.constraints ...

I support that.

@roll
Copy link
Member Author

roll commented Feb 20, 2024

@ezwelty
@peterdesmet
Would it not create very bad duplication with current Table Schema constraints for primitive types (string/integer/etc.)? Historically, Table Schema decided to "copy" JSON Schema constraints instead of just relying on external schemas. For example, we've just added exclusive min/max for numbers.

I agree that the jsonSchema is a constraint literally but what if it's only allowed for object and array types? I would support this. Before I was going to propose jsonSchema property for arrays as well but as a shared constraint I think it's better 👍

@ezwelty
Copy link

ezwelty commented Feb 20, 2024

From an implementation perspective, I presume packages like https://pypi.org/project/jsonschema/ would be used, which support the full JSON Schema spec, so the restriction would be an artificial one, and therefore frustrating to users who run up against it (e.g. trying to use multipleOf). Indeed, it would create two ways to constrain atomic types: JSON Schema way and Frictionless way, but also for array (min/maxLength vs. min/maxItems). I guess this is more a question of branding / mission drift than a technical one.

@roll
Copy link
Member Author

roll commented Feb 20, 2024

@ezwelty
Yes, technically, it's basically free to support it for primitive types but I would be really reluctant to creating basically two major magistral paths for achieving the same thing e.g. field.constraints.maximum: 3 and field.constraints.jsonSchema.maximum: 3. Also with JSON Schema for primitive types you need to duplicate data type. On the other hand, if there are use cases for something like this. Maybe some projects maintain that kind of JSON Schema primitive types so it will benefit them I don't know TBH

@ezwelty
Copy link

ezwelty commented Feb 20, 2024

I agree that the jsonSchema is a constraint literally but what if it's only allowed for object and array types? I would support this.

@roll Then let's go with this and the constraint can always be opened up to other types by feature request.

@roll roll changed the title Added jsonSchema property to object field Added jsonSchema constraint to object and array fields Feb 21, 2024
@roll
Copy link
Member Author

roll commented Feb 21, 2024

Following the discussion the PR has been updated to Added jsonSchema constraint to object and array fields

@roll
Copy link
Member Author

roll commented Feb 21, 2024

ACCEPTED by WG (6/9)

@roll roll merged commit 401ade0 into main Feb 21, 2024
1 check passed
@roll roll deleted the 640/json-schema-constraint branch February 21, 2024 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a json field type and jsonSchema constraint support
3 participants