You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ossie.apache.org by Kyoung Min Kim <ka...@gmail.com> on 2026/10/19 04:02:24 UTC

[DISCUSS] Registering a dialect: the places it has to appear, and a check for them

Hi,

Three PRs this month add a dialect to the core-spec enum, and each one
has needed a follow-up in a different file. #558 (CLICKHOUSE) was
missing the Python enum and the spec.yaml generator's description,
which python-ci and the generator's --check catch, and at its head it
still has no DIALECT_MAP entry, so a CLICKHOUSE body would be parsed
with the default reader. #598 (POSTGRES, REDSHIFT) was missing the
Holistics converter's SQLGLOT_DIALECT table, which JB spotted in
review, and the hex converter's dialect map and snapshot. #607 (TRINO)
has DIALECT_MAP right and is missing the hex and Holistics parts.

As far as I can see a dialect has to appear in seven places: the schema
enum, spec.md, spec.yaml, OssieDialect in python/src/ossie/models.py,
the descriptions in scripts/generate-spec-types.py, DIALECT_MAP in
validation/validate.py, and the SQLGLOT_DIALECT or NOT_SQL table in
converters/holistics/src/ossie_holistics/sqlrefs.py, which mirrors
DIALECT_MAP by its own comment. hex derives its CLI choices from
OssieDialect, so its snapshot test fails on its own when the enum
grows, and its sqlglot mapping ends in assert_never, so an export of a
document in the new dialect stops until a branch is added. Of the
seven, only the Python enum and spec.yaml are checked against the
schema today. A dialect missing from DIALECT_MAP is parsed with
sqlglot's default reader without a word, and one missing from the
Holistics tables is read the same way, and a parseable one is also left
out of its --sql-dialect choices.

I have opened #612 with a few small tests that pin DIALECT_MAP and the
Holistics tables to the schema enum, and the None entries that are not
skipped to ANSI_SQL and the portable language, since a new dialect
mapped to None without a skip entry is the same silent default. Both
already hold on main, and both workflows run on a core-spec change, so
a registration PR that skips either file fails there instead of in
review. The three open dialect PRs would hit one or both tests once
they rebase, which is the reason I am writing before merging anything.

The other half is a short list in CONTRIBUTING.md of what a dialect
registration touches, so the next contributor does not learn it one
review at a time. JB mentioned a CONTRIBUTING.md revision for the spec
change process; if that is still coming, a "registering a dialect"
paragraph could ride along, and I am happy to draft it there or as a
separate PR, whichever is easier.

Thanks,
Kyoung Min Kim

Re: [DISCUSS] Registering a dialect: the places it has to appear, and a check for them

Posted by Chris Eubank via dev <de...@ossie.apache.org>.
Hi all,

Kyoung Min - thanks for the great investigation into all the places where
dialects are present. One thing that jumps out is the difference between
how dialects and vendor extensions are treated in Ossie, which is intended
to be vendor-neutral.

Vendor extensions are clearly opaque to Ossie. The vendor key can be any
string. We've used named vendors in examples, which visually implies an
enum, but we can make those examples more generic. There is also very
little validation of these fields.

Dialects are treated differently. Although we have a portable dialect, many
of the others are vendor-specific. For an expression to be valid, its
dialect must be in the spec's enum. Likewise, while the expression spec
describes these dialects as opaque, the generic validation logic runs
vendor-specific validation for certain dialects. This bakes vendor
awareness deeper into the core spec. While a third party can add their own
vendor extensions freely, adding a new dialect requires going through the
full specification review process.

To JB's point, we're moving the converters, which contain most of the
vendor-specific logic, into the ossie-converters repo. This seems like a
good opportunity to make the core spec as vendor-neutral as possible, so
that adding new dialects or vendors can largely remain scoped to the
converters, without requiring changes to the core spec.

I'll put up a PR and [DISCUSS] thread with a concrete proposal along these
lines.

Kind regards,
Chris

On Sun, Oct 18, 2026 at 9:39 PM Jean-Baptiste Onofré <jb...@nanthrax.net>
wrote:

> Hi Kyoung Min,
>
> Thanks for writing this up, and the "inventory".
> validate.py looks up DIALECT_MAP with .get(), so a missing dialect falls
> back to sqlglot's default reader without a word, and the Holistics
> converter does the same with SQLGLOT_DIALECT, which also drives its
> --sql-dialect choices.
>
> +1 for #612, I would merge it first when ready, and leave a note on #558
> and #607 saying they will go red after a rebase and what is missing, so it
> does not come as a surprise to the contributors.
>
> Two things to keep in mind:
>
> 1. The converters are moving to apache/ossie-converters repository (I will
> do that very very soon, today probably). Once they are there, the Holistics
> test will have to read the schema from the Ossie submodule (REPO_ROOT /
> "ossie" / "core-spec"), and a core-spec PR here will no longer run the
> converter CI. The validation test will still catch a missing DIALECT_MAP
> entry in the registration PR, but the hex and Holistics part will only
> surface when the submodule is bumped in ossie-converters repo. I think that
> is fine, ossie-converters will have its own CONTRIBUTNIG.md and the main
> CONTRIBUTING.md will be updated.
>
> 2. Longer term, I would rather have fewer copies than more tests pinning
> them. The sqlglot reader for each dialect (or the fact that is not SQL)
> could live once in the apache-ossie package next to OssieDIalect, as a
> plain dict of strings so there is no sqlglot dependency. validate.py,
> Holistics and hex would then read it instead of keeping their own tables,
> and SKIP_SQL_VALIDATION would become part of the same table. That is a
> follow-up though, not a reason to hold #612.
>
> For CONTRIBUTING.md, please go ahead with a separate PR: it's a small and
> and useful on its own, and I would rather fold it into the spec change
> process "clarification" later then have it wait for it.
>
> Regards
> JB
>
> On Mon, Oct 19, 2026 at 6:02 AM Kyoung Min Kim <ka...@gmail.com> wrote:
>
> > Hi,
> >
> > Three PRs this month add a dialect to the core-spec enum, and each one
> > has needed a follow-up in a different file. #558 (CLICKHOUSE) was
> > missing the Python enum and the spec.yaml generator's description,
> > which python-ci and the generator's --check catch, and at its head it
> > still has no DIALECT_MAP entry, so a CLICKHOUSE body would be parsed
> > with the default reader. #598 (POSTGRES, REDSHIFT) was missing the
> > Holistics converter's SQLGLOT_DIALECT table, which JB spotted in
> > review, and the hex converter's dialect map and snapshot. #607 (TRINO)
> > has DIALECT_MAP right and is missing the hex and Holistics parts.
> >
> > As far as I can see a dialect has to appear in seven places: the schema
> > enum, spec.md, spec.yaml, OssieDialect in python/src/ossie/models.py,
> > the descriptions in scripts/generate-spec-types.py, DIALECT_MAP in
> > validation/validate.py, and the SQLGLOT_DIALECT or NOT_SQL table in
> > converters/holistics/src/ossie_holistics/sqlrefs.py, which mirrors
> > DIALECT_MAP by its own comment. hex derives its CLI choices from
> > OssieDialect, so its snapshot test fails on its own when the enum
> > grows, and its sqlglot mapping ends in assert_never, so an export of a
> > document in the new dialect stops until a branch is added. Of the
> > seven, only the Python enum and spec.yaml are checked against the
> > schema today. A dialect missing from DIALECT_MAP is parsed with
> > sqlglot's default reader without a word, and one missing from the
> > Holistics tables is read the same way, and a parseable one is also left
> > out of its --sql-dialect choices.
> >
> > I have opened #612 with a few small tests that pin DIALECT_MAP and the
> > Holistics tables to the schema enum, and the None entries that are not
> > skipped to ANSI_SQL and the portable language, since a new dialect
> > mapped to None without a skip entry is the same silent default. Both
> > already hold on main, and both workflows run on a core-spec change, so
> > a registration PR that skips either file fails there instead of in
> > review. The three open dialect PRs would hit one or both tests once
> > they rebase, which is the reason I am writing before merging anything.
> >
> > The other half is a short list in CONTRIBUTING.md of what a dialect
> > registration touches, so the next contributor does not learn it one
> > review at a time. JB mentioned a CONTRIBUTING.md revision for the spec
> > change process; if that is still coming, a "registering a dialect"
> > paragraph could ride along, and I am happy to draft it there or as a
> > separate PR, whichever is easier.
> >
> > Thanks,
> > Kyoung Min Kim
> >
>

Re: [DISCUSS] Registering a dialect: the places it has to appear, and a check for them

Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
Hi Kyoung Min,

Thanks for writing this up, and the "inventory".
validate.py looks up DIALECT_MAP with .get(), so a missing dialect falls
back to sqlglot's default reader without a word, and the Holistics
converter does the same with SQLGLOT_DIALECT, which also drives its
--sql-dialect choices.

+1 for #612, I would merge it first when ready, and leave a note on #558
and #607 saying they will go red after a rebase and what is missing, so it
does not come as a surprise to the contributors.

Two things to keep in mind:

1. The converters are moving to apache/ossie-converters repository (I will
do that very very soon, today probably). Once they are there, the Holistics
test will have to read the schema from the Ossie submodule (REPO_ROOT /
"ossie" / "core-spec"), and a core-spec PR here will no longer run the
converter CI. The validation test will still catch a missing DIALECT_MAP
entry in the registration PR, but the hex and Holistics part will only
surface when the submodule is bumped in ossie-converters repo. I think that
is fine, ossie-converters will have its own CONTRIBUTNIG.md and the main
CONTRIBUTING.md will be updated.

2. Longer term, I would rather have fewer copies than more tests pinning
them. The sqlglot reader for each dialect (or the fact that is not SQL)
could live once in the apache-ossie package next to OssieDIalect, as a
plain dict of strings so there is no sqlglot dependency. validate.py,
Holistics and hex would then read it instead of keeping their own tables,
and SKIP_SQL_VALIDATION would become part of the same table. That is a
follow-up though, not a reason to hold #612.

For CONTRIBUTING.md, please go ahead with a separate PR: it's a small and
and useful on its own, and I would rather fold it into the spec change
process "clarification" later then have it wait for it.

Regards
JB

On Mon, Oct 19, 2026 at 6:02 AM Kyoung Min Kim <ka...@gmail.com> wrote:

> Hi,
>
> Three PRs this month add a dialect to the core-spec enum, and each one
> has needed a follow-up in a different file. #558 (CLICKHOUSE) was
> missing the Python enum and the spec.yaml generator's description,
> which python-ci and the generator's --check catch, and at its head it
> still has no DIALECT_MAP entry, so a CLICKHOUSE body would be parsed
> with the default reader. #598 (POSTGRES, REDSHIFT) was missing the
> Holistics converter's SQLGLOT_DIALECT table, which JB spotted in
> review, and the hex converter's dialect map and snapshot. #607 (TRINO)
> has DIALECT_MAP right and is missing the hex and Holistics parts.
>
> As far as I can see a dialect has to appear in seven places: the schema
> enum, spec.md, spec.yaml, OssieDialect in python/src/ossie/models.py,
> the descriptions in scripts/generate-spec-types.py, DIALECT_MAP in
> validation/validate.py, and the SQLGLOT_DIALECT or NOT_SQL table in
> converters/holistics/src/ossie_holistics/sqlrefs.py, which mirrors
> DIALECT_MAP by its own comment. hex derives its CLI choices from
> OssieDialect, so its snapshot test fails on its own when the enum
> grows, and its sqlglot mapping ends in assert_never, so an export of a
> document in the new dialect stops until a branch is added. Of the
> seven, only the Python enum and spec.yaml are checked against the
> schema today. A dialect missing from DIALECT_MAP is parsed with
> sqlglot's default reader without a word, and one missing from the
> Holistics tables is read the same way, and a parseable one is also left
> out of its --sql-dialect choices.
>
> I have opened #612 with a few small tests that pin DIALECT_MAP and the
> Holistics tables to the schema enum, and the None entries that are not
> skipped to ANSI_SQL and the portable language, since a new dialect
> mapped to None without a skip entry is the same silent default. Both
> already hold on main, and both workflows run on a core-spec change, so
> a registration PR that skips either file fails there instead of in
> review. The three open dialect PRs would hit one or both tests once
> they rebase, which is the reason I am writing before merging anything.
>
> The other half is a short list in CONTRIBUTING.md of what a dialect
> registration touches, so the next contributor does not learn it one
> review at a time. JB mentioned a CONTRIBUTING.md revision for the spec
> change process; if that is still coming, a "registering a dialect"
> paragraph could ride along, and I am happy to draft it there or as a
> separate PR, whichever is easier.
>
> Thanks,
> Kyoung Min Kim
>