Opened 6 years ago

Closed 6 years ago

#3615 closed defect (fixed)

ST_CreateOverview conflict on cast to regclass for duplicate table name

Reported by: hypostase Owned by: strk
Priority: medium Milestone: PostGIS 2.3.0
Component: raster Version: 2.2.x
Keywords: Cc:

Description

I have a series of tables with the same names but in different schemata. When I create an overview using the r_table_schema.r_table_name syntax, I receive the following error PL/pgSQL function st_createoverview(regclass,name,integer,text) line 49 at PERFORM ERROR: relation "o_2_gb_stsu_u_rp75_rd_5m_27700" does not exist CONTEXT: PL/pgSQL function st_createoverview(regclass,name,integer,text) while casting return value to function's return type

The schema in question in not in the search path. When the search path is changed to include the path in the first position, the function works correctly

ST_CreateOverview returns regclass CREATE OR REPLACE FUNCTION ST_CreateOverview(tab regclass, col name, factor int, algo text DEFAULT 'NearestNeighbour') RETURNS regclass AS $$

but the value returned is the overview table name (autocast to regclass) which appears to conflict in the namespace. (where ttab is)

ttab := 'o_'
factor '_' sinfo.tab;

RETURN ttab;

I suspect that this should be

RETURN sinfo.sch
'.' ttab;

or similar, to ensure that the same table is being returned as has been created, when the schema is specified

Attachments (7)

rt_createoverview_schema_expected (371 bytes ) - added by hypostase 6 years ago.
rt_createoverview_schema.sql (1.8 KB ) - added by hypostase 6 years ago.
rtpostgis.sql.in.diff (412 bytes ) - added by hypostase 6 years ago.
rt_createoverview.sql (4.7 KB ) - added by hypostase 6 years ago.
Updated test case sql
rt_createoverview_expected (794 bytes ) - added by hypostase 6 years ago.
Updated test case expected
rt_createoverview.2.sql (4.4 KB ) - added by hypostase 6 years ago.
rt_createoverview_expected.2 (641 bytes ) - added by hypostase 6 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 by strk, 6 years ago

Agreed, schema name needs to be added. Are you willing to provide a patch, including regress test to secure it ?

in reply to:  1 comment:2 by hypostase, 6 years ago

Replying to strk:

Agreed, schema name needs to be added. Are you willing to provide a patch, including regress test to secure it ?

Sure - I'll have a look at the test structure and put something together, I've a bit of other work on at the moment so it may take a few weeks

by hypostase, 6 years ago

by hypostase, 6 years ago

by hypostase, 6 years ago

Attachment: rtpostgis.sql.in.diff added

comment:3 by hypostase, 6 years ago

Attached a diff for the sql.in file and a test and expected.

I've set this as a separate test rather than in the rt_createoverview test and output, so that both happen and it is clear that both happen, but they could also be combined.

comment:4 by strk, 6 years ago

It would be easier to maintain if it was a single test case (combined). Maybe there's a chance to simply run one of the ST_CreateOverview calls in a modified search_path ?

comment:5 by strk, 6 years ago

Resolution: fixed
Status: newclosed

In 15078:

Fix schema support in RT_CreateOveriew

Patch by hypostase
Closes #3615

comment:6 by strk, 6 years ago

I've given a try at the simplified test, see how you like r15078 I think this could be backported to 2.2 branch too

comment:7 by strk, 6 years ago

Resolution: fixed
Status: closedreopened

comment:8 by hypostase, 6 years ago

No worries.

There are three cases - no schema given, schema given with no other table of the same name, and schema given with the same name as the table in the search_path. - I'll do a combined test that checks all. Changing the search path should give you a passing test without the code change. (My original use case has a _lot_ of tables in parent child relationships in different schema used to control which users see what by default)

comment:9 by strk, 6 years ago

Please send a patch making sure all cases are covered, but possibly in the single test. I verified the search_path modification actually did not pass w/out your patch.

by hypostase, 6 years ago

Attachment: rt_createoverview.sql added

Updated test case sql

by hypostase, 6 years ago

Attachment: rt_createoverview_expected added

Updated test case expected

comment:10 by hypostase, 6 years ago

I've added the tests all to the rt_createoverview file. It checks with path set via search_path, and with schema explicitly named. It uses a larger raster for the schema table to ensure the values are different between the original and schema rasters.

comment:11 by strk, 6 years ago

Component: postgisraster
Owner: changed from pramsey to strk
Status: reopenednew

comment:12 by strk, 6 years ago

as the test takes already a long time to execute, could you make the rasters just slighly different ? maybe also reducing the first one. And don't forget to add a DISCARD ALL at the end of the test, so the changes to search_path aren't still in effect on next test

comment:13 by hypostase, 6 years ago

I can offset the second raster instead, and make it much smaller. - The size of the largest overview on the original test is only 3x3 anyway - reducing that significantly would possibly affect the 16 factor test. I can also drop the schema overview factor check, as the thing we're really testing for is that the schema tables can have overviews crated at all.

comment:14 by strk, 6 years ago

Offsetting sounds good to me

by hypostase, 6 years ago

Attachment: rt_createoverview.2.sql added

by hypostase, 6 years ago

comment:15 by hypostase, 6 years ago

This test uses an offset and smaller raster for the schema test to distinguish it, and doesn't create the slowest (2x) overviews. It covers the potential name collisions in both directions, with and without search_path.

comment:16 by strk, 6 years ago

In 15089:

Add complete test for RT_CreateOverview and schemas

Test by hypostase
See #3615

comment:17 by strk, 6 years ago

Resolution: fixed
Status: newclosed

In 15093:

Fix schema support in RT_CreateOveriew

Patch by hypostase
Closes #3615
Include tests

Note: See TracTickets for help on using tickets.