Skip to content

MySQL: Set the character_set_results/character_set_client/character_set_connection session variables on connection setup#334

Merged
phillipleblanc merged 4 commits into
datafusion-contrib:spiceaifrom
Sevenannn:qianqian/mysql-character-set
May 12, 2025
Merged

MySQL: Set the character_set_results/character_set_client/character_set_connection session variables on connection setup#334
phillipleblanc merged 4 commits into
datafusion-contrib:spiceaifrom
Sevenannn:qianqian/mysql-character-set

Conversation

@Sevenannn
Copy link
Copy Markdown
Contributor

@Sevenannn Sevenannn commented May 10, 2025 •
edited by phillipleblanc
Loading

Sets the character_set_results/character_set_client/character_set_connection session variables on connection setup. Also moves the SET time_zone = '+00:00' query into the same setup function.

See MySQL character set results doc for an explanation on these variables: https://dev.mysql.com/doc/refman/8.4/en/charset-connection.html

This is required to ensure that the results we receive from MySQL are encoded correctly into UTF8

@Sevenannn Sevenannn mentioned this pull request May 10, 2025
Merged
@Sevenannn Sevenannn marked this pull request as ready for review May 10, 2025 21:55
phillipleblanc
phillipleblanc approved these changes May 12, 2025
View reviewed changes
Comment thread src/sql/db_connection_pool/mysqlpool.rs Outdated
phillipleblanc
phillipleblanc requested changes May 12, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@phillipleblanc phillipleblanc left a comment

Choose a reason for hiding this comment

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

We shouldn't make this a configuraiton option, we should always set it to utf8mb4

@phillipleblanc phillipleblanc self-requested a review May 12, 2025 07:12
@phillipleblanc phillipleblanc changed the title Support custom mysql_character_set_results configuration in mysql connection pool MySQL: Set the character_set_results/character_set_client/character_set_connection session variables on connection setup May 12, 2025
@phillipleblanc phillipleblanc merged commit d855e2b into datafusion-contrib:spiceai May 12, 2025
3 checks passed
phillipleblanc added a commit that referenced this pull request Jun 5, 2025
@Sevenannn @phillipleblanc
MySQL: Set the character_set_results/character_set_client/`charac…
409fda2 
…ter_set_connection` session variables on connection setup (#334)

* Support custom mysql_character_set_results configuration in mysql connection pool

* MySQL: Set character set to utf8 on connection setup

* Remove error

---------

Co-authored-by: Phillip LeBlanc <phillip@leblanc.tech>
phillipleblanc added a commit that referenced this pull request Jun 5, 2025
@phillipleblanc @sgrebnov @Sevenannn
Merge spiceai changes since 0.4: Custom DuckDB settings, more robust …
d694b4a 
…DuckDB attachment logic, broader type support for DuckDB tables (#364)

* DuckDB: don't use ArrowVTab while creating the table (more types support) (#319)

* Add application_name parameter to postgres options (#335)

* MySQL: Set the `character_set_results`/`character_set_client`/`character_set_connection` session variables on connection setup (#334)

* Support custom mysql_character_set_results configuration in mysql connection pool

* MySQL: Set character set to utf8 on connection setup

* Remove error

---------

Co-authored-by: Phillip LeBlanc <phillip@leblanc.tech>

* Don't display bb8 Postgres pool errors by default (#336)

* Don't visualize Postgres connection pool errors by default.

* Don't display bb8 Postgres pool errors by default

* Make DuckDB database attachments logic more robust (#344)

* Allow custom DuckDB settings to be defined by library users (#362)

* Allow custom DuckDB settings to be defined by library users

* Expose DuckDBSetting trait

* Add builder style with_setting on the registry

* Fix references in docstring

* tweak debug log

* Support connection setup queries for DuckDB

* fix visibility

* fix connection setup

* fix lint

---------

Co-authored-by: Sergei Grebnov <sergei.grebnov@gmail.com>
Co-authored-by: Qianqian <130200611+Sevenannn@users.noreply.github.com>
kashive added a commit to goldsky-io/datafusion-table-providers that referenced this pull request Jul 24, 2025
@kashive @dependabot
@nuno-faria @phillipleblanc @sgrebnov @Sevenannn @xiaom
Update goldsky-main to use Datafusion 47; merge from xiao/pg-sink (#3)
cd6c8a7 
* Bump tempfile from 3.19.1 to 3.20.0 (datafusion-contrib#342)

Bumps [tempfile](https://github.com/Stebalien/tempfile) from 3.19.1 to 3.20.0.
- [Changelog](https://github.com/Stebalien/tempfile/blob/master/CHANGELOG.md)
- [Commits](Stebalien/tempfile@v3.19.1...v3.20.0)

---
updated-dependencies:
- dependency-name: tempfile
  dependency-version: 3.20.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Upgrade datafusion to 47 and datafusion-federation to 0.4.2 (datafusion-contrib#324)

* Upgrade datafusion and datafusion-federation

* Use RemoteTableRef::from instead of RemoteTableRef::try_from

* Upgrade DuckDB to 1.3.0

* Add Rust toolchain to pin rustc/clippy version

---------

Co-authored-by: Phillip LeBlanc <phillip@leblanc.tech>

* DuckDB: don't use ArrowVTab while creating the table (more types support) (datafusion-contrib#319)

* Add application_name parameter to postgres options (datafusion-contrib#335)

* MySQL: Set the `character_set_results`/`character_set_client`/`character_set_connection` session variables on connection setup (datafusion-contrib#334)

* Support custom mysql_character_set_results configuration in mysql connection pool

* MySQL: Set character set to utf8 on connection setup

* Remove error

---------

Co-authored-by: Phillip LeBlanc <phillip@leblanc.tech>

* Don't display bb8 Postgres pool errors by default (datafusion-contrib#336)

* Don't visualize Postgres connection pool errors by default.

* Don't display bb8 Postgres pool errors by default

* Make DuckDB database attachments logic more robust (datafusion-contrib#344)

* Allow custom DuckDB settings to be defined by library users (datafusion-contrib#362)

* Allow custom DuckDB settings to be defined by library users

* Expose DuckDBSetting trait

* Add builder style with_setting on the registry

* Fix references in docstring

* tweak debug log

* Support connection setup queries for DuckDB

* fix visibility

* fix connection setup

* fix lint

* Upgrade dependencies (datafusion-contrib#363)

* Don't pushdown filters with subqueries (datafusion-contrib#316)

* Don't pushdown filters with subqueries.

* Use simpler TreeNode recursion

* bump to 0.5.0

* Bump prost from 0.13.5 to 0.14.0 (datafusion-contrib#367)

Bumps [prost](https://github.com/tokio-rs/prost) from 0.13.5 to 0.14.0.
- [Release notes](https://github.com/tokio-rs/prost/releases)
- [Changelog](https://github.com/tokio-rs/prost/blob/master/CHANGELOG.md)
- [Commits](tokio-rs/prost@v0.13.5...v0.14.0)

---
updated-dependencies:
- dependency-name: prost
  dependency-version: 0.14.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump bollard from 0.19.0 to 0.19.1 (datafusion-contrib#366)

Bumps [bollard](https://github.com/fussybeaver/bollard) from 0.19.0 to 0.19.1.
- [Release notes](https://github.com/fussybeaver/bollard/releases)
- [Changelog](https://github.com/fussybeaver/bollard/blob/master/RELEASE.md)
- [Commits](fussybeaver/bollard@v0.19.0...v0.19.1)

---
updated-dependencies:
- dependency-name: bollard
  dependency-version: 0.19.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump reqwest from 0.12.19 to 0.12.20 (datafusion-contrib#365)

Bumps [reqwest](https://github.com/seanmonstar/reqwest) from 0.12.19 to 0.12.20.
- [Release notes](https://github.com/seanmonstar/reqwest/releases)
- [Changelog](https://github.com/seanmonstar/reqwest/blob/master/CHANGELOG.md)
- [Commits](seanmonstar/reqwest@v0.12.19...v0.12.20)

---
updated-dependencies:
- dependency-name: reqwest
  dependency-version: 0.12.20
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Support for postgres sink

* implement default PostgresWriteConfig

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Nuno Faria <nunofpfaria@gmail.com>
Co-authored-by: Phillip LeBlanc <phillip@leblanc.tech>
Co-authored-by: Sergei Grebnov <sergei.grebnov@gmail.com>
Co-authored-by: Qianqian <130200611+Sevenannn@users.noreply.github.com>
Co-authored-by: Xiao Meng <21000+xiaom@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ewgenius ewgenius ewgenius approved these changes

@phillipleblanc phillipleblanc phillipleblanc approved these changes

@sgrebnov sgrebnov sgrebnov approved these changes

@Jeadie Jeadie Jeadie approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Sevenannn @ewgenius @phillipleblanc @sgrebnov @Jeadie