PostgreSQL-For-Wordpress / postgresql-for-wordpress

A maintained fork of https://wordpress.org/plugins/postgresql-for-wordpress/
GNU General Public License v2.0
228 stars 71 forks source link

_seq appended to table name, how to deal with it properly? #99

Closed SamTyurenkov closed 7 months ago

SamTyurenkov commented 7 months ago

I dont think its a bug, you are probably intentionally appending "_seq" to the end of the table name, however I have some custom tables, and not sure if I should have prepared them for this operation properly?

Is there something I should do specifically to make this statement work on custom table? As you can see I have some translations relation table.

[16-Feb-2024 08:40:55 UTC] SELECT CURRVAL('"wp_options_option_id_seq"')
[16-Feb-2024 08:40:55 UTC] SELECT CURRVAL('"wp_translations_term_relations_seq"')
[16-Feb-2024 08:40:55 UTC] PHP Warning:  pg_query(): Query failed: ERROR:  relation "wp_translations_term_relations_seq" does not exist
LINE 1: SELECT CURRVAL('"wp_translations_term_relations_seq"'...
                       ^ in /var/www/site/web/app/pg4wp/driver_pgsql.php on line 1122

In my specific case this query is executed when Im saving term meta and modifying term links:


public function update_term_language_code( int $term_id, int $tt_id, string $taxonomy )
    {
        if (in_array($taxonomy, \Translations::$non_translatable_taxonomies)) return;

            global $wpdb;

            //Try to get saved language, if its not set - get current language instead
            $object_lang = get_term_meta($term_id, 'translations_term_object_lang', true);
            $object_lang = empty($object_lang) ? \Translations::get_current_language() : $object_lang;

            //Try to get saved source id, if its not set - set current post as source
        if ($object_lang === \Translations::$default_language) {
            $source_id = $term_id;
        } else {
            $source_id = get_term_meta($term_id, 'translations_term_source_id', true);
            $source_id = intval($source_id) > 0 ? $source_id : $term_id;
        }

            //TODO: implement failure logic and maybe retry
            $wpdb->query(
                $wpdb->prepare("INSERT INTO " . $wpdb->prefix . "translations_term_relations (object_id, object_lang, source_id) VALUES (%d, %s, %d) ON DUPLICATE KEY UPDATE object_id=VALUES(object_id), object_lang=VALUES(object_lang), source_id=VALUES(source_id);", $term_id, $object_lang, $source_id)
            );
    }

public static function get_translated_term_id( int $term_id, string $language ): int
    {
        global $wpdb;

        $source_id = intval(get_term_meta($term_id, 'translations_term_source_id', true));

        if ($language !== self::$default_language) {
            $results = $wpdb->get_results(
                $wpdb->prepare("SELECT object_id FROM " . $wpdb->prefix . "translations_term_relations WHERE source_id=%d AND object_lang=%s;", $source_id, $language),
                ARRAY_A
            );

            return empty($results) ? $source_id : $results[0]['object_id'];
        } else {
            return $source_id;
        }
    }

And here is how the tables created:

public function seed_translation_tables()
    {
        global $wpdb;
        require_once ABSPATH . 'wp-admin/includes/upgrade.php';

        $posts_table_name = $wpdb->prefix . 'translations_post_relations';
        $terms_table_name = $wpdb->prefix . 'translations_term_relations';
        $charset_collate  = $wpdb->get_charset_collate();

        $post_sql = "CREATE TABLE IF NOT EXISTS $posts_table_name (
            object_id bigint UNIQUE NOT NULL,
            source_id bigint,
            object_lang varchar(10) NOT NULL,
            PRIMARY KEY (object_id),
            KEY source_id (source_id,object_lang) 
        ) $charset_collate;";

        dbDelta($post_sql);

        $term_sql = "CREATE TABLE IF NOT EXISTS $terms_table_name (
            object_id bigint UNIQUE NOT NULL,
            source_id bigint,
            object_lang varchar(10) NOT NULL,
            PRIMARY KEY (object_id),
            KEY source_id (source_id,object_lang) 
        ) $charset_collate;";

        dbDelta($term_sql);
    }
mattbucci commented 7 months ago

basically this is coming from this function:

https://github.com/PostgreSQL-For-Wordpress/postgresql-for-wordpress/blob/23da9c0dd2808877270838ad13d8da852b212f9e/pg4wp/driver_pgsql.php#L1060

the purpose of CURR_VAL() is to get the last inserted ID which is typically the primary sequence for the table.

if I understand correctly in your table this would be object_id, but I'm not sure that this really is an incrementing sequence.

It seems that object_id is assigned to $term_id per your above code.

wpsqli_insert_id is called somewhere in your code, which should be returning $term_id, but instead is trying to use a sequence value which in your case will fail.

It is possible that PGSQL could account for cases like this by appending "RETURNING id" to the end of insert statements like so: https://github.com/PostgreSQL-For-Wordpress/postgresql-for-wordpress/pull/76#issuecomment-1888449290 but i haven't looked into this in great detail because it's a fairly major change.

A simpler change may be to implement support for LASTVAL(); in the event that we don't have a matching sequence at all rather than trying to assume $table_seq

mattbucci commented 7 months ago

looking into this more last_val is not advisable.

Options are:

mattbucci commented 7 months ago

Draft PR Here: https://github.com/PostgreSQL-For-Wordpress/postgresql-for-wordpress/pull/101