Skip to content

fix: PostgreSQL dialect can not support tinyint type#21445

Merged
metegenez merged 2 commits intoapache:mainfrom
xiedeyantu:smallint
Apr 10, 2026
Merged

fix: PostgreSQL dialect can not support tinyint type#21445
metegenez merged 2 commits intoapache:mainfrom
xiedeyantu:smallint

Conversation

@xiedeyantu
Copy link
Copy Markdown
Member

Which issue does this PR close?

  • No linked issue.

Rationale for this change

DataFusion's PostgreSQL unparser should emit PostgreSQL-compatible SQL for integer casts.Int8 was still being rendered as TINYINT, which is not valid PostgreSQL syntax. This change makes PostgreSQL output SMALLINT instead.

What changes are included in this PR?

  • Added an int8_cast_dtype hook to the SQL unparser dialect abstraction.
  • Updated PostgreSqlDialect to map Int8 to SMALLINT.
  • Routed DataType::Int8 unparsing through the dialect hook.
  • Added a regression test covering select cast(3 as tinyint)] with PostgreSQL unparser output.

Are these changes tested?

  • Yes. Ran:
    • cargo test -p datafusion-sql --test sql_integration cases::plan_to_sql::test_cast_to_tinyint -- --exact
  • The test passes.

Are there any user-facing changes?

  • Yes. PostgreSQL dialect SQL generation now renders CAST(... AS SMALLINT) for Int8 values instead of CAST(... AS TINYINT).

@github-actions github-actions bot added the sql SQL Planner label Apr 7, 2026
Copy link
Copy Markdown
Contributor

@metegenez metegenez left a comment

Choose a reason for hiding this comment

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

Thanks for the work. LGTM. Just adding 1 more test to clear the air would be great.

)
}

#[test]
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.

We can add a test that the default path still produces TINYINT, just for reference and degradation prevention.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done! Thanks!

Copy link
Copy Markdown
Contributor

@metegenez metegenez left a comment

Choose a reason for hiding this comment

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

LGTM. If this does no break any upstream feature, we can merge it. Let's wait a bit for additional reviews.

@xiedeyantu
Copy link
Copy Markdown
Member Author

@metegenez Hi, Do we still need to wait a while longer?

@metegenez metegenez added this pull request to the merge queue Apr 10, 2026
Merged via the queue into apache:main with commit b46634c Apr 10, 2026
35 checks passed
@xiedeyantu
Copy link
Copy Markdown
Member Author

@metegenez Thanks for your review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants