Skip to content

fix: unparse Int8 casts as SIGNED for MySQL dialect - #26883

Merged
alamb merged 1 commit into
apache:mainfrom
hozan23:mysql-unparser-int8-signed
Nov 1, 2026
Merged

alamb merged 1 commit into
apache:mainfrom
hozan23:mysql-unparser-int8-signed

Conversation

@hozan23

@hozan23 hozan23 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

MySQL's CAST does not accept TINYINT as a target type, so an Int8 cast unparsed with MySqlDialect produces SQL that MySQL rejects with ERROR 1064.

See https://dev.mysql.com/doc/refman/8.0/en/cast-functions.html#function_cast

What changes are included in this PR?

MySqlDialect overrides int8_cast_dtype to return SIGNED, like the existing int32_cast_dtype and int64_cast_dtype overrides.

What is the testing strategy for this PR?

New unit test mysql_int8_cast_uses_signed in datafusion/sql/src/unparser/expr.rs.

Are there any user-facing changes?

No API changes.

@github-actions github-actions Bot added the sql SQL Planner label Oct 29, 2026
@hozan23
hozan23 force-pushed the mysql-unparser-int8-signed branch from 431be67 to 689e776 Compare October 29, 2026 12:36

@nuno-faria nuno-faria 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.

Thanks @hozan23.

@alamb
alamb added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 30, 2026
@alamb

alamb commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

@alamb
alamb added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 30, 2026
@hozan23

hozan23 commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @alamb, it seems that the fuzz test didn't fail on the rerun, but the PR was still removed from the queue. Could you please add it again?

@alamb

alamb commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Thanks @alamb, it seems that the fuzz test didn't fail on the rerun, but the PR was still removed from the queue. Could you please add it again?

Thanks
I filed a ticket to track the next fuzz failure

@alamb
alamb added this pull request to the merge queue Nov 1, 2026
Merged via the queue into apache:main with commit 68ac6cc Nov 1, 2026
44 checks passed
Phoenix500526 pushed a commit to Phoenix500526/datafusion that referenced this pull request Nov 3, 2026
…sh_collisions (apache#27034)

## Which issue does this PR close?

- Closes apache#27033.

## Rationale for this change

`completed_groups_release_first_value_bitmap` (added in apache#26782) takes
about 10 minutes in the `cargo test hash collisions (amd64)` merge-queue
job. It aggregates 131,072 distinct keys in one batch. Under
`force_hash_collisions`, every key has the hash 0, so the aggregate is
quadratic. The test checks that `FIRST_VALUE(v ORDER BY v)` releases its
per-batch bitmap, which does not depend on hashing.

The long job gives spot runner shutdowns more time to hit it. Evidence
from merge-queue runs:

- In the passing rerun for apache#26926
([job](https://github.com/apache/datafusion/actions/runs/41571721394/job/125710730373)),
the physical-plan unit test binary took 609.89s. All other tests in it
finished by 03:40:31. This test ran from about 03:40:07 to 03:50:17.
- The median job time went from 6.2 minutes (230 jobs, Oct 21 to Oct 30)
to 12.0 minutes (70 jobs after apache#26782 merged).
- All 5 failed attempts of the job since then were runner shutdowns
while this test was running: apache#26929
([job](https://github.com/apache/datafusion/actions/runs/41114163010/job/124263887540)),
apache#26898
([job](https://github.com/apache/datafusion/actions/runs/41115099549/job/124267135386)),
apache#26883
([job](https://github.com/apache/datafusion/actions/runs/41120437208/job/124285496814)),
apache#26974
([job](https://github.com/apache/datafusion/actions/runs/41559552415/job/125666576696)),
apache#26926
([job](https://github.com/apache/datafusion/actions/runs/41571721394/job/125705896210)).

## What changes are included in this PR?

Skip the test under `force_hash_collisions`, as apache#26594 did for another
test. The imports that only this test uses move into its body, so they
are not unused under the feature.

## What is the testing strategy for this PR?

Test config change only. The regular `cargo test` job still runs the
test. Locally (`--profile ci`), the test takes 0.92s by default and 309s
with `force_hash_collisions`. With the feature, it is now skipped, and
Clippy passes with and without the feature.

## Are there any user-facing changes?

No.
devanbenz pushed a commit to devanbenz/datafusion that referenced this pull request Nov 4, 2026
## Which issue does this PR close?

  - Closes apache#26887.

  ## Rationale for this change

MySQL's `CAST` does not accept `SMALLINT` or the `... UNSIGNED` integer
types as target types, but `arrow_dtype_to_ast_dtype` hard-codes them
for `Int16` and `UInt8`..`UInt64`. As a result, `MySqlDialect` cannot
override them and the generated SQL fails with `ERROR 1064`.

See
https://dev.mysql.com/doc/refman/8.0/en/cast-functions.html#function_cast

Checked against MySQL 8.0.46 and 8.4.13 with `SELECT CAST('1' AS
<type>)`: every type in the issue table fails with `ERROR 1064`, while
`SIGNED` and `UNSIGNED` succeed.

  ## What changes are included in this PR?

- New `Dialect` hooks `int16_cast_dtype` and
`uint8`/`uint16`/`uint32`/`uint64_cast_dtype`. Their defaults return the
previous types.
- `MySqlDialect` returns `SIGNED` for `Int16` and `UNSIGNED` for the
unsigned types.

  This builds on apache#26883.

  ## What is the testing strategy for this PR?

  New unit tests in `datafusion/sql/src/unparser/expr.rs`:
  - `mysql_int16_and_unsigned_casts`: the new MySQL output
- `default_dialect_int16_and_unsigned_casts`: the default output is
unchanged
  - `custom_dialect_int16_and_unsigned_casts`: the builder setters

  ## Are there any user-facing changes?

With `MySqlDialect`, `Int16` casts now unparse as `SIGNED` and unsigned
integer casts as `UNSIGNED`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MySQL unparser emits CAST(x AS TINYINT), which MySQL rejects

3 participants