Skip to content

[Cherry-pick to branch-1.3] [#12518] fix(trino-connector): Keep the engine types when applying projections - #12931

Merged
diqiu50 merged 1 commit into
apache:branch-1.3from
diqiu50:cherry-pick-12520-to-branch-1.3
Sep 4, 2026
Merged

[Cherry-pick to branch-1.3] [#12518] fix(trino-connector): Keep the engine types when applying projections#12931
diqiu50 merged 1 commit into
apache:branch-1.3from
diqiu50:cherry-pick-12520-to-branch-1.3

Conversation

@diqiu50

@diqiu50 diqiu50 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Cherry-pick Information:

  • Original commit: a4bdfda
  • Target branch: branch-1.3
  • Status: ✅ Clean cherry-pick (no conflicts)

…ing projections (apache#12520)

### What changes were proposed in this pull request?

Restore the types the engine assigned to the projected variables when
returning the result of `applyProjection`, instead of forwarding the
types the internal connector derived from its own column handles.

### Why are the changes needed?

Gravitino and the internal connector can describe the same column with
different Trino types. A MySQL tinytext column is an unbounded varchar
in the metadata layer but a varchar(255) in the internal connector, so
pushing a projection down produced an assignment whose two sides
disagreed. Since Trino 444 the planner verifies the types in an
assignment, which turns the disagreement into `Types don't match:
varchar vs varchar(255)` while the query is still being planned.

The normalization is type-agnostic: it does not special-case any column
type, so it also covers `enum` / `set` and any future divergence.
Assignments for columns the internal connector synthesized for
pushed-down expressions have no counterpart among the engine variables
and keep their internal types.

A pre-existing, separate defect is deliberately not addressed here:
`MySQLDataTypeTransformer` truncates the declared precision of MySQL
time/datetime columns to 0 on every Trino version, so selecting such
columns already fails at result serialization today, with or without
pushdown. It will be reported and fixed separately.

`applyAggregation` intentionally stays unchanged: its returned
assignment names are synthesized by the internal connector and have no
engine-side variables to restore types from; the JDBC base connector
already types them with the engine-supplied aggregate output types.

Fix: apache#12518

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- Added `TestGravitinoMetadataApplyProjection` (5 cases, including a
name-collision guard case): without the fix, the two restore cases fail
with the mismatched types.
- Added the `00015_projection_pushdown` tester to the jdbc-mysql test
set: without the fix it reproduces the reported planner error (`Types
don't match: varchar vs varchar(65535)`) on Trino 478; with the fix the
whole jdbc-mysql test set passes (16/16).
- Verified on Trino 443 (which has no assignment verification) that the
change is a harmless normalization: the same queries return identical
results.

---------

Co-authored-by: yuhui <hui@datastrato.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
(cherry picked from commit a4bdfda)
Copilot AI lite review requested due to automatic review settings September 4, 2026 11:14
@diqiu50 diqiu50 self-assigned this Sep 4, 2026

Copilot AI 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.

🟢 Approval recommended

The change is narrowly scoped, preserves existing behavior for synthesized columns, and is covered by both unit and integration tests targeting the reported failure mode.

Pull request overview

Cherry-picks a fix into branch-1.3 for the Trino connector’s applyProjection path so that projected column assignments keep the engine’s declared types (avoiding Trino plan type-mismatch failures when an internal JDBC connector reports a different type for the same underlying column handle).

Changes:

  • Adjust GravitinoMetadata#applyProjection to restore assignment types from the engine’s projection expressions using column-handle-based matching (handles internal variable renames).
  • Add focused unit tests covering restored types, renames, nested expressions, and synthesized columns.
  • Add a MySQL JDBC integration testset reproducing the projection pushdown scenario that previously triggered the type mismatch.
File summaries
File Description
trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoMetadataApplyProjection.java New unit tests validating engine type preservation across several projection/assignment scenarios.
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoMetadata.java Restores engine-assigned types for returned projection assignments, keyed by underlying column handles.
trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00015_projection_pushdown.sql Adds an integration query that exercises projection pushdown with computed columns.
trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00015_projection_pushdown.txt Expected output for the new MySQL projection pushdown testset.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Code Coverage Report

Overall Project 69.12% -0.07% 🟢
Files changed 64.27% 🟢

Module Coverage
aliyun 19.74% 🔴
api 47.62% 🟢
authorization-common 85.96% 🟢
aws 53.54% 🟢
azure 32.1% 🔴
catalog-common 10.4% 🔴
catalog-fileset 80.51% 🟢
catalog-glue 69.24% 🟢
catalog-hive 79.42% 🟢
catalog-jdbc-clickhouse 84.7% 🟢
catalog-jdbc-common 45.47% 🟢
catalog-jdbc-doris 82.61% 🟢
catalog-jdbc-hologres 54.03% 🟢
catalog-jdbc-mysql 79.33% 🟢
catalog-jdbc-oceanbase 78.6% 🟢
catalog-jdbc-postgresql 83.39% 🟢
catalog-jdbc-starrocks 79.16% 🟢
catalog-kafka 77.27% 🟢
catalog-lakehouse-generic 59.35% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.98% 🟢
catalog-lakehouse-paimon 82.17% 🟢
catalog-model 77.99% 🟢
cli 44.51% 🟢
client-java 78.31% 🟢
common 50.74% 🟢
core 83.05% 🟢
filesystem-hadoop3 77.3% 🟢
flink 0.0% 🔴
flink-common 49.22% 🟢
flink-runtime 0.0% 🔴
gcp 32.2% 🔴
hadoop-common 15.43% 🔴
hive-metastore-common 53.77% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 60.29% 🟢
iceberg-rest-server 74.53% 🟢
idp-basic 86.02% +0.67% 🟢
integration-test-common 0.0% 🔴
jobs 66.17% 🟢
lance-common 24.42% 🔴
lance-rest-server 60.13% 🟢
lineage 59.39% 🟢
optimizer 82.95% 🟢
optimizer-api 21.95% 🔴
server 88.75% -0.15% 🟢
server-common 76.72% -1.12% 🟢
spark 28.57% 🔴
spark-common 45.69% 🟢
trino-connector 58.22% -3.12% 🟢
Files
Module File Coverage
idp-basic IdpGroupMetaService.java 95.45% 🟢
IdpRESTUtils.java 79.49% 🟢
server AccessControlNotAllowedFilter.java 100.0% 🟢
StatisticOperations.java 90.8% 🟢
ExceptionHandlers.java 86.71% 🟢
server-common Utils.java 48.08% 🔴
trino-connector GravitinoMetadata.java 25.97% 🔴

@diqiu50
diqiu50 merged commit 60b58ef into apache:branch-1.3 Sep 4, 2026
32 checks passed
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