Change initStatements to charset option when create db connection.

[forfin]

• This option pass to Zend_Db_Adapter_Pdo_Abstract (parent of Mage_Core_Model_Resource_Type_Db_Pdo_Mysql) when init new PDO
• SET NAMES utf8 might be there as a workaround for php < 5.3.6 but should not be a case anymore.
• Ref
  • https://stackoverflow.com/questions/4361459/php-pdo-charset-set-names
  • https://stackoverflow.com/questions/1650591/whether-to-use-set-names

*When Openmage start. It seems to merge every etc/*.xml config including this file. So before this commit, end-user who want to remove initStatements must explicitly set empty to <initStatements /> in later file. Otherwise it still be there.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)
• Add yourself to contributors list

[kiatng]

Have you put this in production? What are the effects/benefits with or without this PR?

[forfin]

Yes, I have used it in production without breaking anythings.

I found this because I was thinking that SET NAMES utf8 statement cause connection pinning in my AWS RDS Proxy env. but seems it not the case.

So, this commit did not breaking anythings and did not solve my original problem either. But 1 lesser query on every connections might be a beneficial for overall.

[kiatng]

Ref https://dev.mysql.com/doc/refman/8.4/en/charset-connection.html

SET NAMES 'charset_name' [COLLATE 'collation_name']

SET NAMES indicates what character set the client uses to send SQL statements to the server. Thus, SET NAMES 'cp1251' tells the server, “future incoming messages from this client are in character set cp1251.” It also specifies the character set that the server should use for sending results back to the client. (For example, it indicates what character set to use for column values if you use a SELECT statement that produces a result set.)

I also read the referenced stackoverflow. I am not sure how it related to our use case. UTF8 has been default in OpenMage since the beginning and we don't have any issues mentioned in the stackoverflow such as threat and wrong encoding in client.

I propose to close this PR unless there is a more compelling reason.

[forfin]

Sorry that I give bad reference.

It start when Openmage create new PDO in Zend_Db_Adapter_Pdo_Abstract

• When init new PDO($dsn, ...); $dsn string are produce by config xml files.
• Before this commit $dsn was

$dsn = "$mysql:model=mysql4;initStatements=SET NAMES utf8;type=pdo_mysql;host=mysql;dbname=openmage;active=1";

• After this commit $dsn will be

$dsn = "$mysql:model=mysql4;charset=utf8;type=pdo_mysql;host=mysql;dbname=openmage;active=1";

• PDO dsn valid options are host, port, dbname, unix_socket, charset for MySQL
• PDO dsn ignore non-existing option so initStatements is ignore.
• charset=xxx option is equivalent to SET NAMES xxx .
  • https://www.php.net/manual/en/ref.pdo-mysql.connection.php
  • SET NAMES utf8 might be there as a workaround for php < 5.3.6 but should not be a case anymore.

So when no initStatements was given, 1 extra query is not call.

magento-lts/app/code/core/Mage/Core/Model/Resource/Type/Db/Pdo/Mysql.php

Lines 33 to 35 in 611bbe7

	if (!empty($configArr['initStatements']) && $conn) {
	$conn->query($configArr['initStatements']);
	}

[Axepih]

Seems OK to me with my tests.
Do you see any problems with this change?

[mattdavenport]

I'm leaning towards merging this for a few reasons:

• Cleaner implementation via DSN construction than a distinct command each connection
• While minimal latency savings, it does save an additional command to the database
• I believe we're under the assumption OpenMage is MySQL/MariaDB only at this point (correct me if I'm wrong), but <charset> is more DB-agnostic than a specific command in <initStatements>

I haven't tested this yet, but will approve once confirmed unless there are other objections.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud