You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ossie.apache.org by "kayemkim (via GitHub)" <gi...@apache.org> on 2026/10/19 03:58:17 UTC
[PR] test(validation): pin the dialect tables in validate.py and the Holistics converter to the schema enum [ossie]
kayemkim opened a new pull request, #612:
URL: https://github.com/apache/ossie/pull/612
## Summary
A dialect added to the core-spec `Dialect` enum also has to be registered in validate.py, as the sqlglot reader that parses it in `DIALECT_MAP` or as `None` plus an entry in `SKIP_SQL_VALIDATION`, and in the Holistics converter's `SQLGLOT_DIALECT` or `NOT_SQL` tables, which mirror those two by their own comment. Nothing checks that today: python-ci compares `OssieDialect` with the schema and validation-ci regenerates spec.yaml, but a dialect missing from `DIALECT_MAP`, or present there as `None` without a `SKIP_SQL_VALIDATION` entry, is parsed with sqlglot's default reader without a word, and one missing from the Holistics tables is read the same way, with a parseable one also left out of its `--sql-dialect` choices. #598 was sent back for `sqlrefs.py`, and #558 and #607 at their current heads each still miss one of these tables.
One commit per side pins the tables to the schema enum, and the `None` entries that are not skipped to `ANSI_SQL` and `OSSIE_SQL_2026`. All of it holds on current main, and both run on a `core-spec/**` change, since validation-ci and the Holistics workflow trigger on it, so a registration PR that skips either file fails there instead of in review.
## Verification
- `validation/tests/`: 238 passed, one of them new. `converters/holistics`: 95 passed, 6 skipped, three of them new.
- Mutations tried in memory: a name added to the schema enum only, the same name added to `DIALECT_MAP` as `None` without a skip entry, a skipped dialect removed from the skip list, and a skipped dialect given a reader. Each fails one assertion, and the set difference names the dialect.
## Related Issues
Discussion on dev@: THREAD_LINK. No issue, the thread is the discussion.
## Checklist
### Validation
- [x] New validation cases are covered by tests
### Tests
- [x] All existing tests pass (`pytest` / CI green)
- [x] New functionality is covered by tests
### Compliance
- [x] ASF license headers are present on all new source files
- [x] No third-party dependencies are added without PMC/IPMC approval
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation): pin the dialect tables in validate.py and the Holistics converter to the schema enum [ossie]
Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on code in PR #612:
URL: https://github.com/apache/ossie/pull/612#discussion_r4313483030
##########
converters/holistics/tests/test_dialect_tables.py:
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""The dialect tables in sqlrefs.py cover the core-spec Dialect enum.
+
+`SQLGLOT_DIALECT` mirrors `DIALECT_MAP` in validation/validate.py and `NOT_SQL`
Review Comment:
Compared directly now in 8188c74, with validate.py loaded the way its own tests do.
##########
validation/tests/test_validate.py:
##########
@@ -349,6 +349,19 @@ def test_holistics_aql_is_skipped_rather_than_parsed_as_sql() -> None:
assert "HOLISTICS_AQL" in _VALIDATE.SKIP_SQL_VALIDATION
+def test_every_schema_dialect_is_mapped_or_skipped(core_schema: dict) -> None:
+ # A dialect added to the schema enum has to be registered here as well, as
+ # the sqlglot reader that parses it or as one sqlglot cannot read. A None
+ # entry that is not skipped is parsed with the default reader, which only
+ # ANSI_SQL and the portable language are meant to be.
+ dialects = set(core_schema["$defs"]["Dialect"]["enum"])
+ parsed_with_default = {d for d, reader in _VALIDATE.DIALECT_MAP.items() if reader is None}
+
+ assert set(_VALIDATE.DIALECT_MAP) == dialects
Review Comment:
Added as a separate test in 8188c74, behind importorskip so the enum check keeps running without sqlglot.
##########
converters/holistics/tests/test_dialect_tables.py:
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""The dialect tables in sqlrefs.py cover the core-spec Dialect enum.
+
+`SQLGLOT_DIALECT` mirrors `DIALECT_MAP` in validation/validate.py and `NOT_SQL`
+mirrors its skip list, so a dialect registered in the schema has to land in one
+of them. Otherwise a body in that dialect is read with sqlglot's default reader,
+and a parseable dialect is missing from the `--sql-dialect` choices.
+"""
+
+from __future__ import annotations
+
+import json
+
+import snapshots
+
+from ossie_holistics import sqlrefs
+
+
+def _schema_dialects() -> set[str]:
+ schema_path = snapshots.REPO_ROOT / "core-spec" / "ossie-schema.json"
+ schema = json.loads(schema_path.read_text(encoding="utf-8"))
+ return set(schema["$defs"]["Dialect"]["enum"])
+
+
+def test_every_schema_dialect_is_parsed_or_marked_not_sql() -> None:
+ assert set(sqlrefs.SQLGLOT_DIALECT) | sqlrefs.NOT_SQL == _schema_dialects()
+
+
+def test_a_dialect_is_either_parsed_or_not_sql() -> None:
+ assert set(sqlrefs.SQLGLOT_DIALECT) & sqlrefs.NOT_SQL == set()
+
+
+def test_only_the_portable_dialects_use_the_default_reader() -> None:
+ default_reader = {
+ d for d, reader in sqlrefs.SQLGLOT_DIALECT.items() if reader is None
+ }
+
+ assert default_reader == {"ANSI_SQL", "OSSIE_SQL_2026"}
Review Comment:
Added in 8188c74 with a plain import, since sqlglot is a runtime dependency here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation): pin the dialect tables in validate.py and the Holistics converter to the schema enum [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #612:
URL: https://github.com/apache/ossie/pull/612#discussion_r4310648756
##########
converters/holistics/tests/test_dialect_tables.py:
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""The dialect tables in sqlrefs.py cover the core-spec Dialect enum.
+
+`SQLGLOT_DIALECT` mirrors `DIALECT_MAP` in validation/validate.py and `NOT_SQL`
Review Comment:
The docstring says `SQLGLOT_DIALECT` mirrors `DIALECT_MAP` and `NOT_SQL` mirrors the skip list, but nothing checks that. Each side is compared only to the schema enum, so the two can disagree and every test still passes. For example, changing `"DATABRICKS": "databricks"` to `"spark"` in `sqlrefs.py` passes all three tests here.
Could we compare them directly?
```python
from importlib.util import module_from_spec, spec_from_file_location
import pytest
def _validate_module():
# validate.py exits at import time without jsonschema.
pytest.importorskip("jsonschema")
path = snapshots.REPO_ROOT / "validation" / "validate.py"
spec = spec_from_file_location("ossie_validate", path)
assert spec is not None and spec.loader is not None
module = module_from_spec(spec)
spec.loader.exec_module(module)
return module
def test_the_tables_match_validate_py() -> None:
validate = _validate_module()
assert sqlrefs.NOT_SQL == validate.SKIP_SQL_VALIDATION
assert sqlrefs.SQLGLOT_DIALECT == {
d: reader
for d, reader in validate.DIALECT_MAP.items()
if d not in validate.SKIP_SQL_VALIDATION
}
```
Also, `converter-holistics-ci.yml` doesn't run on changes under `validation/**`, so a change to `DIALECT_MAP` alone would not run this test.
That said, as the converters will move, it's not a big deal 😄
##########
validation/tests/test_validate.py:
##########
@@ -349,6 +349,19 @@ def test_holistics_aql_is_skipped_rather_than_parsed_as_sql() -> None:
assert "HOLISTICS_AQL" in _VALIDATE.SKIP_SQL_VALIDATION
+def test_every_schema_dialect_is_mapped_or_skipped(core_schema: dict) -> None:
+ # A dialect added to the schema enum has to be registered here as well, as
+ # the sqlglot reader that parses it or as one sqlglot cannot read. A None
+ # entry that is not skipped is parsed with the default reader, which only
+ # ANSI_SQL and the portable language are meant to be.
+ dialects = set(core_schema["$defs"]["Dialect"]["enum"])
+ parsed_with_default = {d for d, reader in _VALIDATE.DIALECT_MAP.items() if reader is None}
+
+ assert set(_VALIDATE.DIALECT_MAP) == dialects
Review Comment:
This checks whether each reader is `None`, but not that a non-`None` reader is a real sqlglot dialect.
A typo like `"bigqeury"` passes. Then `sqlglot.parse_one` raises `ValueError: Unknown dialect 'bigqeury'`, which `validate_sql_expression` doesn't catch, because it only catches `ParseError`, `TokenError` and `RecursionError`.
Could we add a separate test? Keeping it separate means the enum check still runs when sqlglot isn't installed:
```python
def test_every_sqlglot_reader_exists() -> None:
sqlglot = pytest.importorskip("sqlglot")
for reader in _VALIDATE.DIALECT_MAP.values():
if reader is not None:
sqlglot.Dialect.get_or_raise(reader)
```
##########
converters/holistics/tests/test_dialect_tables.py:
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""The dialect tables in sqlrefs.py cover the core-spec Dialect enum.
+
+`SQLGLOT_DIALECT` mirrors `DIALECT_MAP` in validation/validate.py and `NOT_SQL`
+mirrors its skip list, so a dialect registered in the schema has to land in one
+of them. Otherwise a body in that dialect is read with sqlglot's default reader,
+and a parseable dialect is missing from the `--sql-dialect` choices.
+"""
+
+from __future__ import annotations
+
+import json
+
+import snapshots
+
+from ossie_holistics import sqlrefs
+
+
+def _schema_dialects() -> set[str]:
+ schema_path = snapshots.REPO_ROOT / "core-spec" / "ossie-schema.json"
+ schema = json.loads(schema_path.read_text(encoding="utf-8"))
+ return set(schema["$defs"]["Dialect"]["enum"])
+
+
+def test_every_schema_dialect_is_parsed_or_marked_not_sql() -> None:
+ assert set(sqlrefs.SQLGLOT_DIALECT) | sqlrefs.NOT_SQL == _schema_dialects()
+
+
+def test_a_dialect_is_either_parsed_or_not_sql() -> None:
+ assert set(sqlrefs.SQLGLOT_DIALECT) & sqlrefs.NOT_SQL == set()
+
+
+def test_only_the_portable_dialects_use_the_default_reader() -> None:
+ default_reader = {
+ d for d, reader in sqlrefs.SQLGLOT_DIALECT.items() if reader is None
+ }
+
+ assert default_reader == {"ANSI_SQL", "OSSIE_SQL_2026"}
Review Comment:
The Holistics side has the same gap: a misspelled reader on `SQLGLOT_DIALECT` passes all three tests. `sqlrefs.columns` catches only `ParseError`, `TokenError` and `RecursionError`, so the `ValueError` isn't turned into `UnparseableSQL`.
The conversion then crashes instead of reporting an issue. sqlglot is a runtime dependency here, so a plain import works:
```python
import sqlglot
def test_every_reader_is_a_sqlglot_dialect() -> None:
for reader in sqlrefs.SQLGLOT_DIALECT.values():
if reader is not None:
sqlglot.Dialect.get_or_raise(reader)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation,python): pin the dialect and vendor registries to validate.py and the SDK [ossie]
Posted by "christianeu-db (via GitHub)" <gi...@apache.org>.
christianeu-db commented on code in PR #612:
URL: https://github.com/apache/ossie/pull/612#discussion_r4373920223
##########
validation/tests/test_validate.py:
##########
@@ -348,6 +349,49 @@ def test_holistics_aql_is_skipped_rather_than_parsed_as_sql() -> None:
assert "HOLISTICS_AQL" not in _VALIDATE.SQLGLOT_DIALECTS
+def _registered_dialects() -> set[str]:
+ """The tokens in the table of extensions/dialects.md."""
+ registry = Path(__file__).parents[2] / "extensions" / "dialects.md"
+ return {
+ match.group(1)
+ for line in registry.read_text(encoding="utf-8").splitlines()
+ if (match := re.match(r"^\|\s*`([^`]+)`\s*\|", line))
+ }
+
+
+def test_the_dialect_tables_match_the_registry() -> None:
+ # A dialect registered in extensions/dialects.md has to be listed here as
+ # well, as the sqlglot reader that parses it or as one sqlglot cannot read,
+ # or the validator warns about a token the registry says is fine. A dialect
+ # listed here but not in the registry is a registration someone skipped.
+ known = set(_VALIDATE.SQLGLOT_DIALECTS) | _VALIDATE.NON_SQL_DIALECTS
+
+ assert known == _registered_dialects(), (
+ "extensions/dialects.md and SQLGLOT_DIALECTS / NON_SQL_DIALECTS in"
+ " validate.py do not list the same dialects; register a dialect in both"
+ )
Review Comment:
(non-blocking) it might be easier for people adding / modifying the dialects/extensions if we printed the diffs - could be a followup pr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation): pin the dialect tables in validate.py and the Holistics converter to the schema enum [ossie]
Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on PR #612:
URL: https://github.com/apache/ossie/pull/612#issuecomment-6266462097
Pushed 8188c74 with the three. The Holistics test now compares `NOT_SQL` and `SQLGLOT_DIALECT` with `SKIP_SQL_VALIDATION` and `DIALECT_MAP` directly, loading validate.py the way its own tests do, and both sides check every reader with `Dialect.get_or_raise`. The validate.py one is a separate test behind `importorskip("sqlglot")` so the enum check still runs without it. I tried the cases you named on this branch: `"spark"` for DATABRICKS, `bigqeury` on either side and a dropped `DAX` skip entry each fail now, and both suites pass.
On the `validation/**` trigger, agreed it can wait for the move. I'll leave the notes on #558 and #607 and open the CONTRIBUTING.md paragraph as its own PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation,python): pin the dialect and vendor registries to validate.py and the SDK [ossie]
Posted by "kayemkim (via GitHub) via issues" <is...@ossie.apache.org>.
kayemkim commented on code in PR #612:
URL: https://github.com/apache/ossie/pull/612#discussion_r4431243538
##########
validation/tests/test_validate.py:
##########
@@ -348,6 +349,49 @@ def test_holistics_aql_is_skipped_rather_than_parsed_as_sql() -> None:
assert "HOLISTICS_AQL" not in _VALIDATE.SQLGLOT_DIALECTS
+def _registered_dialects() -> set[str]:
+ """The tokens in the table of extensions/dialects.md."""
+ registry = Path(__file__).parents[2] / "extensions" / "dialects.md"
+ return {
+ match.group(1)
+ for line in registry.read_text(encoding="utf-8").splitlines()
+ if (match := re.match(r"^\|\s*`([^`]+)`\s*\|", line))
+ }
+
+
+def test_the_dialect_tables_match_the_registry() -> None:
+ # A dialect registered in extensions/dialects.md has to be listed here as
+ # well, as the sqlglot reader that parses it or as one sqlglot cannot read,
+ # or the validator warns about a token the registry says is fine. A dialect
+ # listed here but not in the registry is a registration someone skipped.
+ known = set(_VALIDATE.SQLGLOT_DIALECTS) | _VALIDATE.NON_SQL_DIALECTS
+
+ assert known == _registered_dialects(), (
+ "extensions/dialects.md and SQLGLOT_DIALECTS / NON_SQL_DIALECTS in"
+ " validate.py do not list the same dialects; register a dialect in both"
+ )
Review Comment:
Done in #670: the message now names the file each missing token belongs in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation): pin the dialect tables in validate.py and the Holistics converter to the schema enum [ossie]
Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on PR #612:
URL: https://github.com/apache/ossie/pull/612#issuecomment-6307724656
Thanks for the pointer. I read #617 and ran this branch on top of it: the two tests that read the schema enum fail with KeyError, the table cross-check and the reader tests still pass. If the enum goes, I would point those two at the table in extensions/dialects.md instead, so the registry carries the check. I put the rest on the list thread and on #617. Whichever lands first, I will do the rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation): pin the dialect tables in validate.py and the Holistics converter to the schema enum [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #612:
URL: https://github.com/apache/ossie/pull/612#discussion_r4347824417
##########
converters/holistics/tests/test_dialect_tables.py:
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
The converters moved to `apache/ossie-converters` repository, so `converters/` no longer exists on `main`.
Could you rebase from `main` and drop this file?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation): pin the dialect tables in validate.py and the Holistics converter to the schema enum [ossie]
Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on code in PR #612:
URL: https://github.com/apache/ossie/pull/612#discussion_r4313483030
##########
converters/holistics/tests/test_dialect_tables.py:
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""The dialect tables in sqlrefs.py cover the core-spec Dialect enum.
+
+`SQLGLOT_DIALECT` mirrors `DIALECT_MAP` in validation/validate.py and `NOT_SQL`
Review Comment:
Compared directly now in ecc467b, with validate.py loaded the way its own tests do.
##########
validation/tests/test_validate.py:
##########
@@ -349,6 +349,19 @@ def test_holistics_aql_is_skipped_rather_than_parsed_as_sql() -> None:
assert "HOLISTICS_AQL" in _VALIDATE.SKIP_SQL_VALIDATION
+def test_every_schema_dialect_is_mapped_or_skipped(core_schema: dict) -> None:
+ # A dialect added to the schema enum has to be registered here as well, as
+ # the sqlglot reader that parses it or as one sqlglot cannot read. A None
+ # entry that is not skipped is parsed with the default reader, which only
+ # ANSI_SQL and the portable language are meant to be.
+ dialects = set(core_schema["$defs"]["Dialect"]["enum"])
+ parsed_with_default = {d for d, reader in _VALIDATE.DIALECT_MAP.items() if reader is None}
+
+ assert set(_VALIDATE.DIALECT_MAP) == dialects
Review Comment:
Added as a separate test in ecc467b, behind importorskip so the enum check keeps running without sqlglot.
##########
converters/holistics/tests/test_dialect_tables.py:
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""The dialect tables in sqlrefs.py cover the core-spec Dialect enum.
+
+`SQLGLOT_DIALECT` mirrors `DIALECT_MAP` in validation/validate.py and `NOT_SQL`
+mirrors its skip list, so a dialect registered in the schema has to land in one
+of them. Otherwise a body in that dialect is read with sqlglot's default reader,
+and a parseable dialect is missing from the `--sql-dialect` choices.
+"""
+
+from __future__ import annotations
+
+import json
+
+import snapshots
+
+from ossie_holistics import sqlrefs
+
+
+def _schema_dialects() -> set[str]:
+ schema_path = snapshots.REPO_ROOT / "core-spec" / "ossie-schema.json"
+ schema = json.loads(schema_path.read_text(encoding="utf-8"))
+ return set(schema["$defs"]["Dialect"]["enum"])
+
+
+def test_every_schema_dialect_is_parsed_or_marked_not_sql() -> None:
+ assert set(sqlrefs.SQLGLOT_DIALECT) | sqlrefs.NOT_SQL == _schema_dialects()
+
+
+def test_a_dialect_is_either_parsed_or_not_sql() -> None:
+ assert set(sqlrefs.SQLGLOT_DIALECT) & sqlrefs.NOT_SQL == set()
+
+
+def test_only_the_portable_dialects_use_the_default_reader() -> None:
+ default_reader = {
+ d for d, reader in sqlrefs.SQLGLOT_DIALECT.items() if reader is None
+ }
+
+ assert default_reader == {"ANSI_SQL", "OSSIE_SQL_2026"}
Review Comment:
Added in ecc467b with a plain import, since sqlglot is a runtime dependency here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation,python): pin the dialect and vendor registries to validate.py and the SDK [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #612:
URL: https://github.com/apache/ossie/pull/612
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation,python): pin the dialect and vendor registries to validate.py and the SDK [ossie]
Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on PR #612:
URL: https://github.com/apache/ossie/pull/612#issuecomment-6407846028
Rebased on main with #617 in. The schema enum is gone, so the two tests now compare `SQLGLOT_DIALECTS` and `NON_SQL_DIALECTS` with the table in extensions/dialects.md, next to the default-reader and reader-exists checks. The vendor pin I mentioned on #617 is here too: `OssieVendor` gains HONEYDEW, POWER_BI and THOUGHTSPOT, vendors.md gains SEMANTIDO, and both SDK enums are compared with their tables. A registry-only change would not have run either suite, so `extensions/**` is added to the validation-ci and python-ci triggers. Registering a dialect is now a three-file change, table plus validate.py plus `OssieDialect`, which is what the thread asked for; if you would rather keep the registry free of the validator, the registry-to-code direction is the one to drop. 274 and 268 passed locally, 33/33 across the matrix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation): pin the dialect tables in validate.py to the schema enum [ossie]
Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on PR #612:
URL: https://github.com/apache/ossie/pull/612#issuecomment-6358376741
Rebased on main after #523 and dropped the Holistics file. The same two tables now live in ossie-converters, and I will pin them there once that repository carries the schema. What stays is the validate.py side: DIALECT_MAP and the skip list against the enum, plus the check that every reader names a sqlglot dialect. 249 passed locally.
Once #617 is in, the enum comparison moves to the table in extensions/dialects.md and the two renamed sets, as said on the list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(validation,python): pin the dialect and vendor registries to validate.py and the SDK [ossie]
Posted by "kayemkim (via GitHub)" <gi...@apache.org>.
kayemkim commented on PR #612:
URL: https://github.com/apache/ossie/pull/612#issuecomment-6460740716
@jbonofre no rush on this one. When you get back to it: with #598 approved and #558 and #607 waiting on a rebase onto #617, landing this before them would let their CI check that each registers the dialect in all three places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org