Working fully qualified table name and tables in different schema from function

[dynajoe]

This is meant as a starting point to address #3.

Here's the example of trigger definition we've been using with temporal_tables native:

SET search_path TO schema_name, public;

CREATE TRIGGER tr_01_table_versioning
BEFORE UPDATE OR DELETE ON table
   FOR EACH ROW EXECUTE PROCEDURE versioning(
     'valid_between', 'schema_name.table_history', true
   );

[dynajoe]

TG_TABLE_NAME isn't enough as it doesn't specify schema.

[ps2goat]

I also noticed that if I used quoted identifiers, such as "Users", TG_TABLE_NAME would be users (lower cased). This would give the error: column "%" of relation "%" does not exist'

This change (and using it elsewhere) appears to fix that for the source table, but not for the history table. I haven't made any changes, yet, to see if I could fix the history table, also.

It's not a big deal, but it is an issue since I'm just starting out with postgres. I have since gone to the standard of lower case and underscores (or, at least, non-quoted identifiers).

[dynajoe]

Using a fully qualified name as the history table should be supported - it is in the extension version.

[dynajoe]

This was necessary because of this error (simplified):

ERROR:  relation "schema_name.table_history" does not exist at character 13
QUERY:  INSERT INTO "schema_name.table_history"(table_id,valid_between) VALUES ($1.table_id,tstzrange($2, $3, '[)'))
CONTEXT:  PL/pgSQL function versioning() line 149 at EXECUTE

[dynajoe]

Any feedback on this PR?

[paolochiodi]

Hi @joeandaverde. As I mentioned on the issue mentioned, I'd like to work on a test case for this before merging the PR: I'm planning to do that during the festivities.

Apart from that, PR seems good to me.

[paolochiodi]

Is there a specific reason for casting history_table to cstring? It appears that the function to_regclass(cstring) doesn't exist.

[dynajoe]

I had the opposite problem:

ERROR:  function to_regclass(text) does not exist at character 8

HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

PostgreSQL 9.5.1 on x86_64-pc-linux-gnu, compiled by gcc (Debian 4.9.2-10) 4.9.2, 64-bit

[dynajoe]

SELECT n.nspname as "Schema",
	  p.proname as "Name",
	  pg_catalog.pg_get_function_result(p.oid) as "Result data type",
	  pg_catalog.pg_get_function_arguments(p.oid) as "Argument data types",
	 CASE
	  WHEN p.proisagg THEN 'agg'
	  WHEN p.proiswindow THEN 'window'
	  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN 'trigger'
	  ELSE 'normal'
	 END as "Type"
	FROM pg_catalog.pg_proc p
	     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace
	WHERE p.proname = 'to_regclass'
	ORDER BY 1, 2, 4;

[paolochiodi]

interesting: docs for 9.6 and 10 state that the parameter should be "text" https://www.postgresql.org/docs/current/static/functions-info.html

Can you try and explicitly cast to text and see what happens?

[dynajoe]

Yeah, that's where I got the error from. Attempting an explicit text cast.

[dynajoe]

This seems to have changed between 9.5 and 9.6:

9.5:

   Schema   |    Name     | Result data type | Argument data types |  Type
------------+-------------+------------------+---------------------+--------
 pg_catalog | to_regclass | regclass         | cstring             | normal

9.6

   Schema   |    Name     | Result data type | Argument data types |  Type
------------+-------------+------------------+---------------------+--------
 pg_catalog | to_regclass | regclass         | text                | normal

Someone mentioning the breaking change:
https://www.postgresql.org/message-id/15698.1480567782%40sss.pgh.pa.us

[paolochiodi]

I've finally been able to put some effort into this. I've incorporated your PR into this branch: https://github.com/nearform/temporal_tables/tree/joeandaverde-master and have made three changes:

• removed cast to cstring that wasn't working
• added tests for tables in another schema
• ported the changes to the "nochecks" version of the trigger

@joeandaverde let me know what you think - I'm happy to merge it

[dynajoe]

@paolochiodi I just added a check for server version number. I considered doing a lookup in pg_class but decided the performance of the pg function is better and there's the possibility of doing it wrong. Such a lookup would need to consider search path and different style of table paths.

[paolochiodi]

@joeandaverde Finally merged all the changes, including your last commit for postgres 9.5 support.

Thank you for you patience.