Skip to content

[#34613] Fix SqlTransformSchemaTransformProvider config schema for cross-language use - #40926

Open
Brijesh-Thakkar wants to merge 1 commit into
apache:masterfrom
Brijesh-Thakkar:fix-34613-sqltransform-config-schema
Open

Brijesh-Thakkar wants to merge 1 commit into
apache:masterfrom
Brijesh-Thakkar:fix-34613-sqltransform-config-schema

Conversation

@Brijesh-Thakkar

Copy link
Copy Markdown

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.Values, 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 IllegalArgumentExceptions 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

Fixes #34613

…he#34613)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 31, 2026 12:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📋 Review Summary

This PR resolves #34613 by replacing Java-specific logical types (EnumerationType and OneOfType) in SqlTransformSchemaTransformProvider.configurationSchema() with portable primitive types (STRING and ARRAY<STRING>) and removing the unused parameters field so the schema can be decoded by non-Java SDKs such as Python. It also adds explicit validation for dialect and tableproviders in from(Row) along with unit tests in both Java and Python.

🔍 General Feedback

  • The migration from EnumerationType to STRING / ARRAY<STRING> with validation in from(Row) preserves type safety and improves error reporting over the previous implementation (which silently ignored unknown table providers).
  • The new Java unit tests (SqlTransformSchemaTransformProviderTest) and Python schema decoding tests (SqlTransformSchemaTransformConfigTest) provide solid coverage for schema portability, configuration validation, and transform expansion.

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

@github-actions

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @jrmccluskey for label python.
Backup reviewer: tvalentyn (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

This pull request likely touches a core component ("core" label). Please review with scrutiny.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

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.

[Bug]: SqlTransformSchemaTransformProvider.java does not work

2 participants