Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zend db returning everything as string #362

Closed
roshanbudhathoki opened this issue Aug 10, 2023 · 8 comments · Fixed by #378
Closed

Zend db returning everything as string #362

roshanbudhathoki opened this issue Aug 10, 2023 · 8 comments · Fixed by #378
Labels
discussion topic is discussed before any action taken

Comments

@roshanbudhathoki
Copy link

data fetch from database, every column is returned as string instead of proper data type, even int is returned as string like "1"

@develart-projects
Copy link
Collaborator

Why is that problem? You are using PHP, not C++.

@develart-projects
Copy link
Collaborator

Anyway, it's bad idea even to touch it, as this is breaking change bringing no value.

@develart-projects develart-projects closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2023
@Jimbolino
Copy link
Collaborator

this is actually recently changed behavior
7c3911c

I think its best to revert this commit

@Jimbolino Jimbolino reopened this Sep 7, 2023
@develart-projects
Copy link
Collaborator

develart-projects commented Sep 7, 2023

@Jimbolino if I remember correctly, PDO fetch was always returning strings. Why is this problem anyway? Are we C++ now?

@develart-projects
Copy link
Collaborator

..or at least was, not sure now. As I'm pretty ignoring types, unless are classes.

@develart-projects
Copy link
Collaborator

YEp, just checked the docs. Before 8.1 all were strings. So now the question is, if we are going to keep framework consistent or we are going to adjust framework to some enforced typing.

Maybe there is a way in the middle: what about to introduce method to disable stringify? Then we have backwards compatibility and also have possibility to go forward with strict types within framework.

@develart-projects develart-projects added the discussion topic is discussed before any action taken label Sep 7, 2023
@Jimbolino
Copy link
Collaborator

afaik since php uses mysqlnd (optional since php 5.3, default in php 5.4) the pdo-mysql driver returns datatypes as they are configured in the database.

I read the migration notes: https://www.php.net/manual/en/migration81.incompatible.php#migration81.incompatible.pdo

  • PDO::ATTR_STRINGIFY_FETCHES now stringifies values of type bool to "0" or "1". Previously bools were not stringified.
    Not sure if mysql has bool fields. I know there are BIT fields. A BIT(1) can be used as a boolean, but they are a pain to work with because they are returned as binary data. You can however have queries that return "true" or "false", but i'm not sure thats a common use case for most people.

  • Integers and floats in result sets will now be returned using native PHP types instead of strings when using emulated prepared statements.
    I'm not sure if zf1 uses "emulated prepared statements", afaik it just concats the strings into a query, and executes it as a regular query statement. But im not sure about that.

In general i'm not a big fan of overwriting defaults to stay compatible with older versions. When a developer upgrades their php version, it is their responsibility to check if their code is compatible with the new version (usually by reading a changelog). By putting these defaults in zf1 we become responsible for these backwards incompatible changes. Usually users dont read changelogs when they do a composer update, at least i only do it on major releases.
In my case i had to debug code that was written in 2019 and survived upgrades from php 7.0 to php 8.2, but recently broke because of this change :)

@hungtrinh please tell us how you see things :)

@hungtrinh
Copy link

hungtrinh commented Sep 8, 2023

@Jimbolino I understand your frustration.

From my point of view, to save time upgrading libraries for many ZF1 based projects, I want to keep everything backward compatible as much as possible

That why I made #324 and #320

To solve your problem i think you can make an PR:

  1. Replace
    if (PHP_VERSION_ID >= 80100) {
    // ensure $config['driver_options'] is an array
    $this->_config['driver_options'] = $this->_config['driver_options'] ?? [];
    if (!isset($this->_config['driver_options'][PDO::ATTR_STRINGIFY_FETCHES])) {
    $this->_config['driver_options'][PDO::ATTR_STRINGIFY_FETCHES] = true;
    }
    }

    By
    if (PHP_VERSION_ID >= 80100 && Zend_Db_Adapter_Pdo_Abstract ::$isPdoStringifyFetchesBackwardCompatiblePhp8) { 
        // ensure $config['driver_options'] is an array 
        $this->_config['driver_options'] = $this->_config['driver_options'] ?? []; 
        if (!isset($this->_config['driver_options'][PDO::ATTR_STRINGIFY_FETCHES])) { 
            $this->_config['driver_options'][PDO::ATTR_STRINGIFY_FETCHES] = true; 
        } 
    } 
  2. Add to line 57 public static attribute public static $isPdoStringifyFetchesBackwardCompatiblePhp8 = true;
    abstract class Zend_Db_Adapter_Pdo_Abstract extends Zend_Db_Adapter_Abstract
    {
    /**
    * Transaction in BC mode for php >= 8 flag
    *
    * Bring back behavior of PDO::rollback()/PDO::commit()
    * after an implicit commit like php before ver 8
    * (Don't throw PDOException with message 'There is no active transaction' )
    *
    * @see https:/php/php-src/commit/990bb34891c83d12c5129fd781893704f948f2f4
    */
    public static $isTransactionInBackwardCompatibleMode = true;

So in your use case in zf project folder public/index.php after line zend autoloading library add this line to solve your problem
That will keep your project in forward compatible with php >= 8

... \\after Zend Framework Autoload library line
Zend_Db_Adapter_Pdo_Abstract::$isPdoStringifyFetchesBackwardCompatiblePhp8) = false;

Why use static attribute why not use normal property support setting per connection?

Because Project base on ZF1 we have 2 use case only

  • Backward Compatible With Php <= 8
  • Forward Compatible With Php >= 8

Use static property flag save time to setting for project in 1 line at 1 startup point.
If we provide normal class attribute flag lead to enable mixed mode: some where in project $isPdoStringifyFetchesBackwardCompatiblePhp8=false other $isPdoStringifyFetchesBackwardCompatiblePhp8=true => it's not safe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion topic is discussed before any action taken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants