Opened 2 years ago

Closed 2 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 2 years ago.
rt_createoverview_schema.sql (1.8 KB) - added by hypostase 2 years ago.
rtpostgis.sql.in.diff (412 bytes) - added by hypostase 2 years ago.
rt_createoverview.sql (4.7 KB) - added by hypostase 2 years ago.
Updated test case sql
rt_createoverview_expected (794 bytes) - added by hypostase 2 years ago.
Updated test case expected
rt_createoverview.2.sql (4.4 KB) - added by hypostase 2 years ago.
rt_createoverview_expected.2 (641 bytes) - added by hypostase 2 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 2 years ago by strk

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

comment:2 in reply to:  1 Changed 2 years ago by hypostase

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

Changed 2 years ago by hypostase

Changed 2 years ago by hypostase

Changed 2 years ago by hypostase

Attachment: rtpostgis.sql.in.diff added

comment:3 Changed 2 years ago by hypostase

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 Changed 2 years ago by strk

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 Changed 2 years ago by strk

Resolution: fixed
Status: newclosed

In 15078:

Fix schema support in RT_CreateOveriew

Patch by hypostase
Closes #3615

comment:6 Changed 2 years ago by strk

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 Changed 2 years ago by strk

Resolution: fixed
Status: closedreopened

comment:8 Changed 2 years ago by hypostase

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 Changed 2 years ago by strk

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.

Changed 2 years ago by hypostase

Attachment: rt_createoverview.sql added

Updated test case sql

Changed 2 years ago by hypostase

Attachment: rt_createoverview_expected added

Updated test case expected

comment:10 Changed 2 years ago by hypostase

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 Changed 2 years ago by strk

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

comment:12 Changed 2 years ago by strk

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 Changed 2 years ago by hypostase

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 Changed 2 years ago by strk

Offsetting sounds good to me

Changed 2 years ago by hypostase

Attachment: rt_createoverview.2.sql added

Changed 2 years ago by hypostase

comment:15 Changed 2 years ago by hypostase

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 Changed 2 years ago by strk

In 15089:

Add complete test for RT_CreateOverview and schemas

Test by hypostase
See #3615

comment:17 Changed 2 years ago by strk

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.