GeoTools

Setting PK_METADATA_TABLE for JDBCDatastore without schema ends in illegal statement

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 8.0-RC2
  • Fix Version/s: 8.7, 9.0-RC1
  • Component/s: jdbc
  • Labels:
    None
  • Environment:

Description

To recognize autoincrement on primary columns in oracle (sequence + insert trigger) I use the pk metatable capabilities of geotools. Because of naming conventions this table needs to be renamed so I specified the custom name (without schema name) via property as followed:

Find datastore with custom pk metatable
Map<String, Object> params = new HashMap<String, Object>();
params.put(JDBCDataStoreFactory.DBTYPE.key, "oracle");
params.put(JDBCJNDIDataStoreFactory.JNDI_REFNAME.key, DATASOURCE_JNDI_NAME);
params.put(JDBCDataStoreFactory.PK_METADATA_TABLE.key, "CUSTOM_TABLE_NAME"); //table name without schema
JDBCDataStore datastore = (JDBCDataStore)DataStoreFinder.getDataStore(params);

The resultant statement looks like this: SELECT * FROM CUSTOM_TABLE_NAME.GT_PK_METADATA

The reason is located in JDBCDataStoreFactory:

org.geotools.jdbc.JDBCDataStoreFactory.java:199
String metadataTable = (String) PK_METADATA_TABLE.lookUp(params);
if(metadataTable != null)  {
	MetadataTablePrimaryKeyFinder tableFinder = new MetadataTablePrimaryKeyFinder();
	if(metadataTable.contains(".")) {
		//...
	} else {
		tableFinder.setTableSchema(metadataTable); // THIS IS WRONG
		// should be: tableFinder.setTableName(metadataTable);
	}
	//...
}

Instead of setting the table name the schema name gets modified.

I hope somebody can fix it quickly because setting the schema prior as configuration is not applicable for me (jndi connection configuration allows change of database without touching source code).

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andrea Aime added a comment -
The only way to get a quick fix is to propose a patch with a unit test yourself, this normally gets the patch applied within a few weeks, otherwise you'll have to wait for a volounteer to have spare time and decide to fix this particular issue among the others contained in this ticket tracker.
Show
Andrea Aime added a comment - The only way to get a quick fix is to propose a patch with a unit test yourself, this normally gets the patch applied within a few weeks, otherwise you'll have to wait for a volounteer to have spare time and decide to fix this particular issue among the others contained in this ticket tracker.
Hide
Permalink
Robin Pradel added a comment -
Thanks for your reply. I will try to create a patch. Are there any specifications about the unit test or is it simply a before - after indicator?
Show
Robin Pradel added a comment - Thanks for your reply. I will try to create a patch. Are there any specifications about the unit test or is it simply a before - after indicator?
Hide
Permalink
Andrea Aime added a comment -
Look at PostgisPrimaryKeyFinderTest and PostgisPrimaryKeyFinderTestSetup in the modules/library/jdbc module, those have to modified to add the extra test. Each database module in modules/plugin/jdbc extends those two classes to do the database specific portion of the test.
I guess you will have to explicitly tell the db to use a specific metadata table instead of the default one, that might be a bit tricky... or just force manually the default name, that might trigger the bug regardless.
Show
Andrea Aime added a comment - Look at PostgisPrimaryKeyFinderTest and PostgisPrimaryKeyFinderTestSetup in the modules/library/jdbc module, those have to modified to add the extra test. Each database module in modules/plugin/jdbc extends those two classes to do the database specific portion of the test. I guess you will have to explicitly tell the db to use a specific metadata table instead of the default one, that might be a bit tricky... or just force manually the default name, that might trigger the bug regardless.
Hide
Permalink
Robin Pradel added a comment -
I fixed the code in this patch including the test. It would be nice if somebody can apply it. Thanks.
Show
Robin Pradel added a comment - I fixed the code in this patch including the test. It would be nice if somebody can apply it. Thanks.
Hide
Permalink
Justin Deoliveira added a comment -
Patch looks ok to me. Nitpicky thing, can you rename the getDataStoreFactoryParams() to "createDataStoreFactoryParams()" in the interest of keeping subclass hooks named in a consistent way.
Show
Justin Deoliveira added a comment - Patch looks ok to me. Nitpicky thing, can you rename the getDataStoreFactoryParams() to "createDataStoreFactoryParams()" in the interest of keeping subclass hooks named in a consistent way.
Hide
Permalink
Robin Pradel added a comment -
Ok. I have fully rebuilt the patch and renamed the method. The new patch replaces the old one in this ticket.
Show
Robin Pradel added a comment - Ok. I have fully rebuilt the patch and renamed the method. The new patch replaces the old one in this ticket.
Hide
Permalink
Andrea Aime added a comment -
Ah, sorry this one feel under the radar. I've just applied the patch on master, 9.x and 8.x
Show
Andrea Aime added a comment - Ah, sorry this one feel under the radar. I've just applied the patch on master, 9.x and 8.x

People

  • Assignee:
    Andrea Aime
    Reporter:
    Robin Pradel
Vote (0)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: