Opened 13 years ago
Closed 13 years ago
#532 closed defect (fixed)
Temporary table geography columns appear in other's sessions
|Reported by:||Mike Taves||Owned by:||pramsey|
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.).
Change History (17)
comment:1 by , 13 years ago
comment:2 by , 13 years ago
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)?
comment:3 by , 13 years ago
what about just using pg_table_is_visible(oid)?
by , 13 years ago
comment:4 by , 13 years ago
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:
- It is for PostgreSQL 8.2 and up
- I removed the filter
c.relkind IN('r','v')figuring that
t.typname = 'geography'is enough, but I'm no expert on this
comment:5 by , 13 years ago
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?
by , 13 years ago
comment:6 by , 13 years ago
This patch seems to work right for me on pgsql 8.3 and 8.4. Everyone else happy with it?
comment:7 by , 13 years ago
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.
comment:8 by , 13 years ago
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);
comment:9 by , 13 years ago
|Status:||new → closed|
comment:10 by , 13 years ago
comment:11 by , 13 years ago
|Priority:||medium → blocker|
|Status:||closed → reopened|
Yuck - r5947 is totally wrong. This should be a release blocker for 1.5.2.
comment:12 by , 13 years ago
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
comment:13 by , 13 years ago
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)));
comment:14 by , 13 years ago
I think I must have just missed this commit. Anyhow 1.5 now matches trunk and seems to provide the correct behavior, yes?
comment:15 by , 13 years ago
|Status:||reopened → closed|
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
This can be resolved by using the SQL statement (modified from CREATE OR REPLACE VIEW geography_columns):
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.)