Brijesh-Thakkar opened a new pull request, #40926: URL: https://github.com/apache/beam/pull/40926
## Summary `SqlTransformSchemaTransformProvider` (`schematransform:org.apache.beam:sql_transform:v1`) could not be discovered from Python: its configuration schema used Java-only logical types that Python cannot decode, so `typing_from_runner_api` / `named_tuple_from_schema` failed with `No logical type registered for URN ...`. The offending fields were: - `dialect`: `EnumerationType` (`beam:logical_type:javasdk_enum:v1`) - `tableproviders`: `ARRAY<EnumerationType>` (`javasdk_enum:v1`) - `parameters`: `OneOfType` (`beam:logical_type:javasdk_oneof:v1`) This PR makes the schema use only portable types: - `dialect` is now a nullable `STRING`. `from()` rejects values that are not a registered query planner (currently `calcite`). - `tableproviders` is now a nullable `ARRAY<STRING>`. `from()` rejects names that are not the table type of a registered `TableProvider`. The old enum enforced this implicitly, and the old code silently ignored unknown names. - `parameters` is removed. It was never read by `from()` or `expand()` (there was only a TODO), so no functionality is lost. It can return later using plain types once it is actually implemented. ## Why not a Python-side workaround The earlier attempt in #34605 changed only the Python wrapper and was closed unmerged; its author noted it could not work because of this issue. The schema itself has to be fixed. ## Compatibility The config schema changes shape for Java callers: anyone who built a config `Row` with `EnumerationType.Value`s, or set `parameters`, must now pass plain strings and drop `parameters`. The discussion on #34613 found no users of this provider. Validation is still enforced, now in `from()` with a clear `IllegalArgumentException` that lists the allowed values. YAML's `Sql` transform (`yaml/standard_providers.yaml`) passes only `query`, so it should be unaffected. I read `yaml_mapping.py`, `yaml_join.py` and `yaml_combine.py` but did **not** run the YAML tests. ## Testing Java (`:sdks:java:extensions:sql:expansion-service:test --tests '*SqlTransformSchemaTransformProvider*'`, and `:check`): 6 tests pass. They cover: - the config schema has only portable types and no `parameters`; - an invalid dialect is rejected; - an unknown table provider is rejected; - a valid config is accepted; - expansion with `dialect=calcite`; - expansion with `ddl` (a `CREATE EXTERNAL TABLE ... TYPE 'text'` over a temp file, then a query reading from it). Note that DDL table types resolve only with `autoload=true`; with `autoload=false` the `tableproviders` entries are registered as named schemas. That is existing behavior and is unchanged here. With only `SqlTransformSchemaTransformProvider.java` reverted to master, the 5 original tests fail: `testConfigurationSchemaUsesOnlyPortableTypes` fails on its assertion (`parameters` is present), and the other 4 fail with `ClassCastException` (`String cannot be cast to EnumerationType$Value`) while building the config `Row`, because the old enum fields reject plain strings. That is the same non-portability as the bug. With the fix they pass. The DDL test was added afterwards and was not run against master. Manual cross-language check (not automated in CI): I built the expansion-service shadow jar locally and drove it from Python. - Discovery succeeds (previously it failed to decode the schema). - Expansion with `dialect='calcite'` works and returns `output` and `errors`. - `dialect='bogus'` and `tableproviders=['nope']` raise clear `IllegalArgumentException`s through Python. - I only expanded the transform; I did not execute a pipeline. Python (`sql_test.py`): `SqlTransformSchemaTransformConfigTest` is a regression guard, **not** an end-to-end test. It hand-builds the proto that the Java schema emits and checks that `named_tuple_from_schema` decodes it, and that the old enum logical type still does not decode. It does not start an expansion service, and the Java side is pinned by the Java test, so the two copies must be kept in sync by hand. `pytest apache_beam/transforms/sql_test.py`: 2 passed (the other 11 tests in the file are existing cross-language tests that skip without a runner). yapf and ruff are clean; `spotlessApply` / `spotlessCheck` are clean. ## Not addressed `ExternalTransformProvider` still skips this transform by default: the identifier `schematransform:org.apache.beam:sql_transform:v1` has no `beam:` prefix, so it does not match the default URN pattern (`^beam:schematransform:org.apache.beam:([\w-]+):(\w+)$`). It works with a custom `urn_pattern`. I left it out because `yaml/standard_providers.yaml` depends on the identifier. A follow-up issue will be filed. Generated with [Claude Code](https://claude.com/claude-code) Fixes #34613 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
