Temporary table geography columns appear in other's sessions

[Mike Taves]

If you have multiple sessions, and from only one:

create temp table foo(id serial primary key, geog geography(Point, 4326));

Now all sessions "see" the column in geography_columns, even though it is not "their" temporary table. Other sessions can also create the same temporary table concurrently, but are distinguished by different f_table_schema values (pg_temp_3, pg_temp_4, etc.).

[Mike Taves]

This can be resolved by using the SQL statement (modified from CREATE OR REPLACE VIEW geography_columns):

	SELECT
		current_database() AS f_table_catalog, 
		n.nspname AS f_table_schema, 
		c.relname AS f_table_name, 
		a.attname AS f_geography_column,
		geography_typmod_dims(a.atttypmod) AS coord_dimension,
		geography_typmod_srid(a.atttypmod) AS srid,
		geography_typmod_type(a.atttypmod) AS type
	FROM 
		pg_class c, 
		pg_attribute a, 
		pg_type t, 
		pg_namespace n
	WHERE t.typname = 'geography'
	AND a.attisdropped = false
	AND a.atttypid = t.oid
	AND a.attrelid = c.oid
	AND c.relnamespace = n.oid
	AND (NOT c.relistemp OR NOT pg_is_other_temp_schema(n.oid));

Note: I have removed c.relkind IN('r','v') from the original definition since I'm not sure if you can have that kind mixed in with t.typname = 'geography'. Furthermore, pg_is_other_temp_schema(oid) appeared in Postgres 8.2 (judging from when it appeared in ​http://www.postgresql.org/docs/8.2/static/functions-info.html the documentation.)

[mcayland]

This seems reasonably sensible - do we also have the same problem with geometry as it stands at the moment? For 1.5 we support a minimum version of PostgreSQL 8.3 for the geography typmod so I don't see this being a problem.

Otherwise, any chance you could post a diff (diff -u or diff -c) against latest 1.5 so that reviewers can see the exact change(s)?

Mark.

[kneufeld]

what about just using pg_table_is_visible(oid)?

​http://www.postgresql.org/docs/8.4/static/functions-info.html#FUNCTIONS-INFO-SCHEMA-TABLE

[Mike Taves]

I've added the patch. I tried pg_table_is_visible(oid), but it seemed to be false (I'm not sure why).

Also, before considering this patch, be aware:

1. It is for PostgreSQL 8.2 and up
2. I removed the filter c.relkind IN('r','v') figuring that t.typname = 'geography' is enough, but I'm no expert on this

[mcayland]

I've just had a look at this on PostgreSQL 8.3 and I don't see a relistemp column in pg_class - which PostgreSQL version are you working on?

Mark.

[pramsey]

Using pg_rel_is_visible

[pramsey]

This patch seems to work right for me on pgsql 8.3 and 8.4. Everyone else happy with it?

[pramsey]

Nevermind, pg_rel_is_visible() won't pick up tables that you are allowed to see, but that aren't currently in your search path. We need a better patch. I also confirmed that relistemp is a 8.4-only attribute of pg_class. So, currently not seeing a solution to the problem, will continue to look.

[pramsey]

OK, got it:

CREATE OR REPLACE VIEW geography_columns AS
	SELECT
		current_database() AS f_table_catalog, 
		n.nspname AS f_table_schema, 
		c.relname AS f_table_name, 
		a.attname AS f_geography_column,
		geography_typmod_dims(a.atttypmod) AS coord_dimension,
		geography_typmod_srid(a.atttypmod) AS srid,
		geography_typmod_type(a.atttypmod) AS type
	FROM 
		pg_class c, 
		pg_attribute a, 
		pg_type t, 
		pg_namespace n
	WHERE t.typname = 'geography'
        AND a.attisdropped = false
        AND a.atttypid = t.oid
        AND a.attrelid = c.oid
        AND c.relnamespace = n.oid
        AND NOT pg_is_other_temp_schema(c.relnamespace);

[Mike Taves]

Any reason why r5947 (branches/1.5) and r5948 (trunk) are different?

[mcayland]

Yuck - r5947 is totally wrong. This should be a release blocker for 1.5.2.

[robe]

Yah that is wrong. How about replacing pg_table_is_visible(c.oid) with

has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER'::text)

I checked information_schema.tables and that is what it uses and I verified it can see temp tables I created. Its also listed in 8.3 docs so I think safe to use ​http://www.postgresql.org/docs/8.3/static/functions-info.html

[robe]

Actually Trunk is right as Mike noted and Paul has listed up there -

AND NOT pg_is_other_temp_schema(c.relnamespace);

I thought has_table_privledge… would be enough, but super user can see tables created in user temp spaces and of course they have rights to those though (since I can select by qualifying with temp space). If I launch 2 different sessions with non-super user and with same user account — I can't see the temp tables created in the other session.

I do question though — why do we want people seeing tables they don't have rights to again? I think we had the discussion before. Note Paul's above still allows a person to see tables they don't have rights to.

So I really think it should be:

AND has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER'::text) 
    AND  NOT pg_is_other_temp_schema(c.relnamespace)

which is actually what information_schema.tables definition has Below is the definition of information_schema.tables in case I missed anything.

SELECT (current_database())::information_schema.sql_identifier AS table_catalog, (nc.nspname)::information_schema.sql_identifier AS table_schema, (c.relname)::information_schema.sql_identifier AS table_name, (CASE WHEN (nc.oid = pg_my_temp_schema()) THEN 'LOCAL TEMPORARY'::text WHEN (c.relkind = 'r'::"char") THEN 'BASE TABLE'::text WHEN (c.relkind = 'v'::"char") THEN 'VIEW'::text ELSE NULL::text END)::information_schema.character_data AS table_type, (NULL::character varying)::information_schema.sql_identifier AS self_referencing_column_name, (NULL::character varying)::information_schema.character_data AS reference_generation, (CASE WHEN (t.typname IS NOT NULL) THEN current_database() ELSE NULL::name END)::information_schema.sql_identifier AS user_defined_type_catalog, (nt.nspname)::information_schema.sql_identifier AS user_defined_type_schema, (t.typname)::information_schema.sql_identifier AS user_defined_type_name, (CASE WHEN ((c.relkind = 'r'::"char") OR ((c.relkind = 'v'::"char") AND (EXISTS (SELECT 1 FROM pg_rewrite WHERE (((pg_rewrite.ev_class = c.oid) AND (pg_rewrite.ev_type = '3'::"char")) AND pg_rewrite.is_instead))))) THEN 'YES'::text ELSE 'NO'::text END)::information_schema.yes_or_no AS is_insertable_into, (CASE WHEN (t.typname IS NOT NULL) THEN 'YES'::text ELSE 'NO'::text END)::information_schema.yes_or_no AS is_typed, (CASE WHEN (nc.oid = pg_my_temp_schema()) THEN 'PRESERVE'::text ELSE NULL::text END)::information_schema.character_data AS commit_action FROM ((pg_namespace nc JOIN pg_class c ON ((nc.oid = c.relnamespace))) LEFT JOIN (pg_type t JOIN pg_namespace nt ON ((t.typnamespace = nt.oid))) ON ((c.reloftype = t.oid))) 
WHERE (((c.relkind = ANY (ARRAY['r'::"char", 'v'::"char"])) AND (NOT pg_is_other_temp_schema(nc.oid))) AND ((pg_has_role(c.relowner, 'USAGE'::text) OR has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER'::text)) OR has_any_column_privilege(c.oid, 'SELECT, INSERT, UPDATE, REFERENCES'::text)));

[pramsey]

I think I must have just missed this commit. Anyhow 1.5 now matches trunk and seems to provide the correct behavior, yes?

[robe]

Close enough. I still think we should not be showing tables at all that people do not have permission to, but perhaps that is too much of change in behavior for 1.5