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

[BUG]: Column::TYPE_BINARY and Column::TYPE_TINYINTEGER are both 26 #16532

Closed
jturbide opened this issue Feb 18, 2024 · 6 comments
Closed

[BUG]: Column::TYPE_BINARY and Column::TYPE_TINYINTEGER are both 26 #16532

jturbide opened this issue Feb 18, 2024 · 6 comments
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium

Comments

@jturbide
Copy link
Contributor

Describe the bug
Phalcon\Db\Column::TYPE_BINARY and Phalcon\Db\Column::TYPE_TINYINTEGER should not be equal

To Reproduce

Column::TYPE_BINARY === Column::TYPE_TINYINTEGER // true

Expected behavior

Column::TYPE_BINARY === Column::TYPE_TINYINTEGER // false

Additional context
Column::TYPE_BINARY is not numeric
Column::TYPE_TINYINTEGER is numeric
Missing unit test for TYPE_BINARY

@jturbide jturbide added bug A bug report status: unverified Unverified labels Feb 18, 2024
@jturbide
Copy link
Contributor Author

jturbide commented Feb 18, 2024

The biggest issue is when we are using casting (i.e. castOnHydrate) it will convert my binary into a int because it found the wrong type. Since I don't want to turn off casting on hydrate, and want to fix the bind type, tis is my temporary solution for now. I made my own Mysql adapter to get around the issue affected by the wrong Column type value.

use Phalcon\Db\Column;

class Mysql extends \Phalcon\Db\Adapter\Pdo\Mysql
{
    public function describeColumns(string $table, string $schema = null): array
    {
        $definitions = parent::describeColumns($table, $schema);
        
        if (Column::TYPE_TINYINTEGER !== Column::TYPE_BINARY) {
            return $definitions;
        }
        
        foreach ($definitions as $key => $definition) {
            
            if ($definition->getType() === Column::TYPE_TINYINTEGER && !$definition->isNumeric()) {
                // probably a binary at this point
                
                $newDefinition = [];
                
                // protected to public
                $prefix = chr(0).'*'.chr(0);
                foreach ((array)$definition as $k => $value) {
                    $newDefinition[str_replace($prefix, '', $k)] = $value;
                }
                
                $newDefinition['bindType'] = Column::BIND_PARAM_BLOB;
                $newDefinition['type'] = Column::TYPE_VARBINARY;
                unset($newDefinition['scale']);
                
                // reset definition
                $definitions[$key] = new Column($definition->getName(), $newDefinition);
            }
        }
        
        return $definitions;
    }
}

jturbide added a commit to zemit-cms/core that referenced this issue Feb 20, 2024
@niden niden mentioned this issue Apr 5, 2024
5 tasks
@niden niden self-assigned this Apr 5, 2024
@niden niden added status: medium Medium 5.0 The issues we want to solve in the 5.0 release and removed status: unverified Unverified labels Apr 5, 2024
@niden niden added this to Phalcon v5 Apr 5, 2024
@niden niden moved this to In Progress in Phalcon v5 Apr 5, 2024
@niden
Copy link
Member

niden commented Apr 5, 2024

This has been fixed in #16564

Thank you @jturbide

@niden niden closed this as completed Apr 5, 2024
@niden niden mentioned this issue Apr 5, 2024
5 tasks
@niden niden moved this from In Progress to Implemented in Phalcon v5 May 6, 2024
@dunarri
Copy link

dunarri commented Jan 12, 2025

Hi, according to the release notes for version 5.7.0 this fix was included, however I've installed 5.7.0 and still have the issue.
Here is the code I used to reproduce it:

<?php
echo 'Using Phalcon '.phpversion('phalcon').PHP_EOL;

// Check column type values according to the original cause of issue 16532
use Phalcon\Db\Column;
var_dump(Column::TYPE_BINARY);
var_dump(Column::TYPE_TINYINTEGER);
var_dump(Column::TYPE_BINARY === Column::TYPE_TINYINTEGER);

class Test extends Phalcon\Mvc\Model
{
    public $id;
    public $binary_field;

    public function initialize()
    {
        $this->setSource('test');   // provide actual table name
    }
}

// Create a DI and setup the database connection
$di = new Phalcon\Di\FactoryDefault();
$di->setShared('db', function () {
    return new Phalcon\Db\Adapter\Pdo\Mysql([
        'host'     => 'localhost',
        'dbname'   => 'phalcon_binary_test',
        'username' => 'phalcon',
        'password' => 'secret',
    ]);
});
Phalcon\Di\Di::setDefault($di);

// Enable castOnHydrate
Phalcon\Mvc\Model::setup(['castOnHydrate' => true]);

$model = Test::findFirst(1);
var_dump($model->binary_field);
?>

And this is the result:

Using Phalcon 5.7.0
int(26)
int(26)
bool(true)
int(0)

I installed Phalcon from the remi repo using yum:

$ yum list installed | grep phalcon
php81-php-phalcon5.x86_64           5.7.0-1.el7.remi              @remi-safe    

@niden
Copy link
Member

niden commented Jan 15, 2025

@dunarri
Copy link

dunarri commented Jan 18, 2025

Should I install through PECL instead of yum and remi repo?

@niden
Copy link
Member

niden commented Jan 18, 2025

You can give it a try. I am pretty sure that Remi already has v5.8 but in any case, give it a go with PECL and let us know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium
Projects
Status: Implemented
Development

No branches or pull requests

3 participants