Opened 15 years ago

Last modified 15 years ago

#72 closed task (fixed)

ST_Estimated_Extent sometimes returns null if table exists but not in current schema

Reported by: robe Owned by: robe
Priority: medium Milestone: PostGIS 1.4.0
Component: postgis Version: 1.4
Keywords: Cc:

Description

What steps will reproduce the problem?

  1. Create a schema - say myschema
  2. SET search_path to public,myschema;
  3. SELECT ST_Estimated_Extent('somegeomtable', 'the_geom');

where the somegeomtable is in myschema schema.

What is the expected output? Either an error or the estimated extent

What do you see instead? NULL

Note: If SET search_path to myschema, public then estimated extent is correctly returned

If SET search_path to public

Then it correctly returns relation 'soemgeomtable' does not exist.

So only seems to be an issue if schema is in search path but not the first schema in search path.

Seems to happen in both 1.3.3 and 1.3.4 SVN.

Change History (8)

comment:1 by mcayland, 15 years ago

Hmmm. The problem with this is that 'SELECT current_schema()' returns the first named schema within the search_path, and so any other schemas are ignored :(

The only fix for this will be to add an extra schema parameter to st_estimated_extent() so that it will take parameters in the form of (schema, table, column) and deprecate the 2 parameter version. I don't mind doing the work, however is it possible to clearly mark functions as deprecated in the documentation to dissuade people from using the old version?

ATB,

Mark.

comment:2 by robe, 15 years ago

Mark,

I'm not sure its worth fixing this well at least not for 1.3.4 at any rate. Just thought I would mention it since it came up in my testing of verifying the docs were accurate. The docs are pretty clear that this version only searches the current_schema (Not search path).

What I was expecting was for it to throw an error like in the st_estimated_extent(schema,table,geomname) case when you feed it a schema that is not the schema the table resides in rather than just throwing null.

Wouldn't changing the function to

CREATE OR REPLACE FUNCTION st_estimated_extent(text, text)

RETURNS box2d AS $$

SELECT st_estimated_extent(current_schema(), $1,$2); $$

LANGUAGE 'sql' IMMUTABLE STRICT SECURITY DEFINER;

Suffice. Then it will correctly throw an error. Otherwise you are left wandering if you just don't have any stats rather than you were searching the wrong schema.

comment:3 by kneufeld, 15 years ago

I would advise using pg_catalog.pg_table_is_visible(oid) rather than current_schema() to determine which schema a table is located in. I agree that the problem here is that current_schema only returns the first entry in the search_path. It looks like we have the same problem with addgeometrycolumns and most like, other functions that use the current_schema().

IE. postgis=# CREATE TABLE tmp2.foo();

postgis=# SET search_path to tmp, tmp2, public;

postgis=# SELECT n.nspname AS schemaname postgis-# FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace postgis-# WHERE c.relkind IN ('r',) postgis-# AND n.nspname NOT IN ('pg_catalog', 'pg_toast') postgis-# AND pg_catalog.pg_table_is_visible(c.oid) postgis-# AND c.relname = 'foo';

schemaname


tmp2

(1 row)

comment:4 by robe, 15 years ago

I'm sure Paul is probably grunting profusely at this point and wishing he released while you were on vacation. So you mean something like this?

CREATE OR REPLACE FUNCTION st_estimated_extent(param_table_name text,param_geom_name text)

RETURNS box2d AS $$

DECLARE var_schema text; BEGIN

SELECT n.nspname AS schemaname INTO var_schema FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind IN ('r',) AND n.nspname NOT IN ('pg_catalog', 'pg_toast')

AND pg_catalog.pg_table_is_visible(c.oid)

AND c.relname = quote_ident(param_table_name);

IF FOUND THEN

RETURN st_estimated_extent(var_schema, param_table_name, param_geom_name);

ELSE

RAISE EXCEPTION 'Table % is not visible.', param_table_name ;

END IF;

END; $$

LANGUAGE 'plpgsql' IMMUTABLE STRICT SECURITY DEFINER;

comment:5 by kneufeld, 15 years ago

:) Yes, I'm sure he is.

Your function looks great. I haven't tested it, mind you, but I think the logic here looks better than using current_schema(). My 2 cents.

I might change ('r',) to ('r') and add a limit 1. (I just cut and pasted the query from psql using the -E parameter … nice feature).

If you wanted to get really fancy, you might even try adding a HINT message indicating the user try extending the search_path or use the 3 param version. I'm not sure how to accomplish this in plpgsql, but Mark figured out how to do it using the ereport() function call.

comment:6 by mcayland, 15 years ago

I do like Kevin's solution of using the pg_table_is_visible() function - I think it was me that last altered that section of code and I must admit it's the first time I've come across this function.

There is no need to complicate things further by adding a plpgsql wrapper though - simply alter the SQL queries used in the C code, as the majority of the work is done by passing SQL query strings to the SPI directly anyway.

ATB,

Mark.

comment:7 by robe, 15 years ago

Don't quite get you mean by no need to complicate things by adding the plpgsql wrapper .

I had started by stuffing your subquery instead of current_schema in the sql wrapper version and that worked except for the case when the subquery returns no records, then it comes back with the same annoying null estimated_extent with no message as to why. Which was my main gripe to begin with.

You mean touch the C code directly? Hmm haven't looked at the C code.

comment:8 by mcayland, 15 years ago

Added quick fix to 1.3 branch and trunk, although too late for 1.3.4 release (and 1.3.x should be kept stable regardless). Bumping milestone to 1.4.

ATB,

Mark.

Note: See TracTickets for help on using tickets.