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

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

Closed
SamTyurenkov opened this issue Feb 16, 2024 · 3 comments
Closed

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

SamTyurenkov opened this issue Feb 16, 2024 · 3 comments

Comments

@SamTyurenkov
Copy link

SamTyurenkov commented Feb 16, 2024

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
Copy link
Collaborator

basically this is coming from this function:

function wpsqli_get_primary_sequence_for_table(&$connection, $table)

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: #76 (comment)
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
Copy link
Collaborator

looking into this more last_val is not advisable.

Options are:

  • Add an incrementing sequence to your table
  • We could add a way for tables to mark that like "term_relationships", wpsqli_insert_id becomes a noop
  • We could not do a query for the last inserted ID at all if no sequence is defined on the table
  • [Preferred] We could refactor to always append RETURNING LAST_INSERTED_ID to all insert queries, then store this value during execution in a global which wpsqli_insert_id can use, removing the need for the sequence lookup entirely

@mattbucci
Copy link
Collaborator

Draft PR Here: #101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants