-
Notifications
You must be signed in to change notification settings - Fork 33
Expose ogr_fdw_info functions via SQL #260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This returns the SQL for foreign table creation for a given OGR layer. It accepts the server name, layer name, table name, and options for laundering table and column names.
Returns a list of layers in the specified FDW server.
|
I think this is useful but I'm +0 on the API.
Design goal is basically to provide the affordances of the existing software, but removing the need for locally installed software, yes? |
Yes, so you don't need ogr-fdw built+installed on both the client and the PG server. And also to fix random schema issues by getting the SQL since
👍 Happy to iterate.
Yeah, that's what I was leaning towards.
I feel like it might get confusingly overloaded? ogr_fdw_info(datasource text) returns text; -- CREATE SERVER...
ogr_fdw_info(server oid, layer text) returns text; -- CREATE FOREIGN TABLE...How about: ogr_fdw_info_server(ogr_datasource text, [...options]) returns text; -- CREATE SERVER...
ogr_fdw_info_table(server oid, layer text, [...options]) returns text; -- CREATE FOREIGN TABLE...
ogr_fdw_info_layers(ogr_datasource text) returns setof(record); -- layer list for connection string
ogr_fdw_info_layers(server oid) returns setof(record); -- layer list for existing FDW server
I guess there's some potential security questions here too if we're executing from ogr datasource strings? Since calling server-side functions is potentially a different class of permissions than doing |
This latter won't work so great I don't thing, as the text form will shadow the oid form (since most people access the oid type by feeding in a text string and letting the auto-cast do its magic.
Yeah, I hadn't thought about the extent to which the FDW API was providing some free security cover. I mean, we could just make the |
I kinda assumed it'd be the difference between: But maybe there's some special TIL typing coercion that happens by default? I mean, (I'll accept "it's still too confusing, don't do it" 😁 )
ok. Maybe |
Nope I just have my head up my ass. Ignore me and procede. |
What is the expected output of ogr_fdw_info_layers? If it's simply a set of layer names, I think output being a
makes more sense. |
|
I've been somewhat bogged down in other stuff, but I'll get back to finishing this soon hopefully :-)
TIL So, yes it does 👍 |
|
SETOF text or SETOF anytype will return multiple records and will just have the function as the name of column if no alias is provided. The main difference is SETOF record or RETURNS TABLE would allow multiple columns per row but you have to deal with that messy business of using RETURNS TABLE (defining the columns) or using OUT params, which is kinda pointless if you are only returning one column. Take for example the postgis function ST_SubDivide, it returns a SETOF geometry and you use it like any other table function. But that said, I don't think there is any performance penalty doing it your way, just that you have a slightly longer definition. |
|
Getting back around to this...
Unsurprisingly, it turns out I am wrong 😁 And while there's various Only similar thing I could find is What would you prefer for the a. take a text arg which uses the arg to lookup a foreign server by name, and falls back to using it as an OGR datasource string |
|
I guess we must accept the server name as text... |
|
Kinda stalled out. Almost there functionally I think? It's just less coolio than I had hoped ;) |
What cool thing were you hoping for? Taking server name as not text but a real fdw_server object. or the other cool thing you spoke about ogr_read, which I assume is the cool thing I'm looking for which is similar to a dblink like function that takes a connection and and sql and returns a set of RECORD or a JSON object I know you frowned on that cause you really wanted to be able for it to unravel the structure, but that feature of undefined I find pretty useful to handle types that don't map cleanly to PostgreSQL or where my app expects the same structure which is gasp JSON, cause most web apps output JSON anyway these days.. So I would want a function that can output JSON anyway even if you had a function that returned a completely structured output. I use dblink a lot to connect to other postgresql servers since postgres_fdw lacks that functionality to do adhoc queries. Would be cool to be able to pass adhoc ogr_fdw_sql statements like you said. But I'd probably want it to be called something like ogr_read_as_table, ogr_read_as_json But that said, that wish should probably be separate from this pull request anyway to prevent scope creep. |
|
I guess I was hoping to be able to address things as |
I've found it useful to be able to introspect OGR datasources as the PostgreSQL server can see it — SQL equivalents of
ogr_fdw_infocommands.Add a function to list available OGR layers from an FDW server:
Add a function to get the
CREATE FOREIGN TABLESQL for a particular OGR layer, the same asIMPORT FOREIGN SCHEMAuses. This can help for issues where OGR is reporting column types wrongly, or some columns aren't needed but it's mostly correct.ogr_fdw_table_sql(server_name, ogr_layer_name, table_name=NULL, launder_column_names=TRUE, launder_table_name=TRUE)Not sure on preferences whether you'd prefer the functions to take OIDs (eg:
SELECT ogr_fdw_layers(myserver);) instead of names; and suggestions also very welcome on improvements for naming or docs.