Skip to content

Testing latest version of source code and numeric types are not converted any more #109

@hahnn

Description

@hahnn

Hello,

I had a little bit of time on this sunday and decided to start testing all the changes implemented in the last version of the source code (for v3.4).

It seems to me there are some kind of regressions if i don't do any mistake, and that's related to the transformation of all numeric types.

I take this example:

---------------------
[1710079211.6086] Error running :
CREATE TABLE IF NOT EXISTS wp_wpforms_logs (
                        id BIGINT(20) NOT NULL AUTO_INCREMENT,
                        title VARCHAR(255) NOT NULL,
                        message LONGTEXT NOT NULL,
                        types VARCHAR(255) NOT NULL,
                        create_at DATETIME NOT NULL,
                        form_id BIGINT(20),
                        entry_id BIGINT(20),
                        user_id BIGINT(20),
                        PRIMARY KEY (id)
                ) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_520_ci
---- converted to ----
CREATE TABLE IF NOT EXISTS wp_wpforms_logs (
                        id bigserial,
                        title VARCHAR(255) NOT NULL,
                        message text NOT NULL,
                        types VARCHAR(255) NOT NULL,
                        create_at timestamp NOT NULL,
                        form_id BIGINT(20),
                        entry_id BIGINT(20),
                        user_id BIGINT(20),
                        PRIMARY KEY (id)
                );
----> ERREUR:  erreur de syntaxe sur ou près de « ( »
LINE 7:    form_id BIGINT(20),

The BIGINT(20) has not been replaced by simply BIGINT in the create table above.

As far as I can see, in CreateTableSQLRewriter.php source code file, in public function rewrite(): string, there is a call to:

$sql = $this->rewrite_numeric_type($sql);

And my understanding is that this function is supposed to convert all numeric types as PostgreSQL known types.

However, it seems to me this function will analyze the CREATE TABLE statements to replace ONLY the numeric types with an AUTO-INCREMENT keyword. So by the way, all standard numeric types are not handled.

I think that's why in the resulting conversion, the line:

id BIGINT(20) NOT NULL AUTO_INCREMENT,

has been correctly converted to (however the NOT NULL clause disappeared whereas it should remain):

id bigserial,

But the lines:

                        form_id BIGINT(20),
                        entry_id BIGINT(20),
                        user_id BIGINT(20),

have not been converted at all and remain exactly the same, whereas they should have been converted to:

                        form_id bigint,
                        entry_id bigint,
                        user_id bigint,

So, the rewrite_numeric_type($sql) function should be able to take care of additional numeric type conversions, and not only the auto_increment ones.

Maybe adding the code below just before the return $sql; line at the end of the function would convert all what is not converted:

        // bigint
        $pattern = '/bigint(\(\d+\))?([ ]*NOT NULL)?[ ]*/i';
        preg_match($pattern, $sql, $matches);
        if($matches) {
            $sql = preg_replace($pattern, 'bigint', $sql);
        }

        // int
        $pattern = '/int(\(\d+\))?([ ]*NOT NULL)?[ ]*/i';
        preg_match($pattern, $sql, $matches);
        if($matches) {
            $sql = preg_replace($pattern, 'integer', $sql);
        }

        // smallint
        $pattern = '/smallint(\(\d+\))?([ ]*NOT NULL)?[ ]*/i';
        preg_match($pattern, $sql, $matches);
        if($matches) {
            $sql = preg_replace($pattern, 'smallint', $sql);
        }

        // Handle for numeric and decimal -- being replaced with serial
        $numeric_patterns = ['/numeric(\(\d+\))?([ ]*NOT NULL)?[ ]*/i', '/decimal(\(\d+\))?([ ]*NOT NULL)?[ ]*/i'];
        foreach($numeric_patterns as $pattern) {
            preg_match($pattern, $sql, $matches);
            if($matches) {
                $sql = preg_replace($pattern, 'integer', $sql);
            }
        }

However the addition of this code above to the function is incorrect because it will convert again what has been converted before. For example, what I mean is that when the line:

form_id BIGINT(20),

Is converted to:

form_id bigint,

then it is converted again to:

form_id biginteger,

just because the word bigint contains the word int as well...

One last thing is that the tinyint case is not treated in this function. I think there should be this piece of code somewhere:

        // tinyint
        $pattern = '/ tinyint(\(\d+\))/i';
        preg_match($pattern, $sql, $matches);
        if($matches) {
            $sql = preg_replace($pattern, ' smallint', $sql);
        }

And probably remove also this line at the very begining of the source code file (which I've commented out here below):

        // For flash-album-gallery plugin
        // ' tinyint'           => ' smallint'
    ];

What do you think?

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions