Skip to content

Change vendor_name from enum to free-form string - #91

Merged
khush-bhatia merged 3 commits into
apache:mainfrom
jochenchrist:vendor-name-free-form-string
May 21, 2026
Merged

Change vendor_name from enum to free-form string#91
khush-bhatia merged 3 commits into
apache:mainfrom
jochenchrist:vendor-name-free-form-string

Conversation

@jochenchrist

@jochenchrist jochenchrist commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Change vendor_name in custom extensions from a closed enum to a free-form string, allowing any vendor or organization to define custom extensions without modifying the core spec
  • Existing vendor names (COMMON, SNOWFLAKE, SALESFORCE, DBT, DATABRICKS) are preserved as examples in the JSON Schema and documented as well-known examples in the spec
  • Updated spec.yaml, osi-schema.json, and spec.md

🤖 Generated with Claude Code

@jbonofre
jbonofre self-requested a review March 23, 2026 09:41

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

osi-schema.json should be updated on line 20-26 as it still references the array.
It also still mentions Supported vendors for custom extensions (enumeration definition) which is not accurate anymore.

In the spec.md, the custom extensions section (line 359) still show:

  custom_extensions:                                                                                                                                                                                                                                                           
    - vendor_name: string  # Must be from vendors enum 

which is not accurate with the change.
Also CustomExtension in schema still references $ref: "#/$defs/Vendor". While the Vendordef itself is updated, the overallvendors` top-level property and its description remain inconsistent.

If I understand the intent (relaxing vendors), and it makes to me, I suggest to update the vendors ref across the three files to be consistent.

Can you please do a complete pass to update all enum references ?

@jochenchrist

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I've addressed the feedback:

  • Updated the vendors array description in osi-schema.json from "enumeration definition" to a functional description
  • Fixed the spec.md custom extensions schema comment to say "Free-form string identifying the vendor" instead of "Must be from vendors enum"

All remaining enum references should now be consistent with the change to a free-form string.

@jbonofre

Copy link
Copy Markdown
Member

@jochenchrist Many thanks ! I'm doing a new pass.

@khush-bhatia

khush-bhatia commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

The change makes sense, but would end up being a breaking change.

Let's use git tag for versioning and then it would still be nice to have static shareable links for the different versions of the spec via build automation tooling ? @jbonofre what do you think ?

@jbonofre

Copy link
Copy Markdown
Member

@khush-bhatia I agree. We have version: 0.1.1 in the spec. I would say that any breaking change on the spec (like this one) should bump the version (according to semver).
We have branches per version, however, I would recommend to use only two active spec versions (so branches) because the maintenance is "burden" 😄

I can work on the "CI" to verify and manage the versions.

Thoughts ?

@khush-bhatia

Copy link
Copy Markdown
Contributor

@jochenchrist Thanks for this PR. We have done the release work. And now this PR can be merged to main. Do you mind rebasing this PR to the latest main ?

jochenchrist and others added 3 commits May 21, 2026 08:49
Allow any vendor or organization to define custom extensions without
requiring changes to the core specification. Existing vendor names
(COMMON, SNOWFLAKE, SALESFORCE, DBT, DATABRICKS) are preserved as
examples.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update vendors array description in osi-schema.json to reflect
  non-exhaustive examples instead of enumeration definition
- Update spec.md custom extensions schema comment from "Must be from
  vendors enum" to "Free-form string identifying the vendor"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jochenchrist
jochenchrist force-pushed the vendor-name-free-form-string branch from 4aa1c09 to b1f67b5 Compare May 21, 2026 06:49
@jochenchrist

Copy link
Copy Markdown
Contributor Author

Thanks @khush-bhatia! Rebased onto latest main. Resolved the conflict in osi-schema.json / spec.yaml by keeping GOODDATA in the vendor examples list. Ready to merge.

@khush-bhatia
khush-bhatia merged commit 53c446e into apache:main May 21, 2026
sfc-gh-kbhatia added a commit that referenced this pull request May 28, 2026
These top-level enumeration properties were not referenced by any example
or downstream consumer. validate.py only reads dialects from each
expression's per-dialect entries via the Expression definition. Removing
them keeps the root schema focused on version and semantic_model. The
Dialect and Vendor $defs are retained for use within Expression and
custom extensions.

Also drop the now-orphaned Vendors enumeration table from spec.md
(vendor_name has been a free-form string since #91; the table was a
holdover) and refresh the stale "Must be one of the values from 'vendors'
enum above" comments in spec.yaml so they describe vendor_name as a
free-form vendor identifier.

.... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code)

Co-Authored-By: Cortex Code <noreply@snowflake.com>
sfc-gh-kbhatia added a commit that referenced this pull request May 28, 2026
These top-level enumeration properties were not referenced by any example
or downstream consumer. validate.py only reads dialects from each
expression's per-dialect entries via the Expression definition. Removing
them keeps the root schema focused on version and semantic_model. The
Dialect and Vendor $defs are retained for use within Expression and
custom extensions.

Also reorganize the Vendors content in spec.md: it no longer fits under
the Enumerations section because vendor_name has been a free-form string
since #91. Moved it under Custom Extensions as a "Vendor Names"
subsection listing well-known examples. Refreshed the stale "Must be one
of the values from 'vendors' enum above" comments in spec.yaml so they
describe vendor_name as a free-form vendor identifier.

.... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code)

Co-Authored-By: Cortex Code <noreply@snowflake.com>
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