Postgrest 8 fails to start if standard_conforming_strings=off #1992
Comments
|
I reformatted your post a litle bit to make it easier to read. A few thoughts for now:
Unrelated to this issue, but just looking at the query output, I'm thinking whether we could avoid to send all the comments in that SQL query to the database. If we could strip comments at compile time, that would be cool.
This seems like a reasonable solution that we could add to the docs.
Maybe we could add some CI tests to be able to run our test-suite against different kinds of postgresql configurations down the road to investigate.
Every |
wolfgangwalther
added
bug
ci
|
The side-effect I'm referring to is that everything executed via postgrest will be done with Thus all triggers, functions, views, etc it executes inside its session will be done with the 'wrong' setting relative to how the logic is potentially expecting. If it ends up executing something in our application logic with a \ in a string that is not an E'' string, it will probably not work. I'm still confirming how big an issue that is for us specifically, but it does mean my workaround is probably not a generic workaround for the issue. The session settings of the
Yes, but it is only being done once for each connection in the pool - not for every request. In any case, internally postgresql is doing a set_config based on the role setting I did anyway. So whether the role sets the setting or postgrest does won't be terribly different bar the round trip time to do the statement from postgrest. But as I fear it is not a good solution anyway, it is probably a bit mute. Anyway, putting everything postgrest does into |
|
For the specific query that it is currently failing on, changing the 2 lines to be would fix that query regardless of the |
Thanks for the explanation. I haven't really looked at what
Correct,
We do set the search path on every request - and would do so with this setting, too. The role-based setting is done server-side only and should not impact performance in the same way. I don't think we have machinery to do session-based |
We could use |
Pretty sure that's the only place where we use backslashes for a query. So seems like a simple fix. |
How would that help with stripping SQL comments? |
I've noted that QuasiQuoters strip comments, can't find a direct reference to that behavior though. |
Tried that - didn't work. Did so on one of the other queries in |
Confirmed, see PR. |
Ah, my bad there. I remember at a point we had a quasiquoted SQL and noted that it had comments which didn't appear in the db logs. Perhaps it was a different library or had a workaround. Edit: I think it was this one(old version) postgrest/src/PostgREST/DbStructure.hs Lines 400 to 405 in 9847e60 |
|
It looks like the fix for #1938 assumes Interestingly, the code for pgFmtLit in SqlFragment.hs seems to use E'' strings, but pgFmtArrayLit does not although I don't know Haskell so could be reading it incorrectly. |
I don't think
|
|
I have tried Please find the logs below while starting the service,
Also experimentally changed the standard_conforming_strings postgreSQL configuration for postgREST database user, but failed.
Whenever I reverts the version to Thanks. |
|
@rinshadka Can you confirm that error happens when running the below query? (Replace pfkSourceColumns querywith recursive
pks_fks as (
-- pk + fk referencing col
select
conrelid as resorigtbl,
unnest(conkey) as resorigcol
from pg_constraint
where contype IN ('p', 'f')
union
-- fk referenced col
select
confrelid,
unnest(confkey)
from pg_constraint
where contype='f'
),
views as (
select
c.oid as view_id,
n.nspname as view_schema,
c.relname as view_name,
r.ev_action as view_definition
from pg_class c
join pg_namespace n on n.oid = c.relnamespace
join pg_rewrite r on r.ev_class = c.oid
where c.relkind in ('v', 'm') and n.nspname = ANY($1 || $2)
),
transform_json as (
select
view_id, view_schema, view_name,
-- the following formatting is without indentation on purpose
-- to allow simple diffs, with less whitespace noise
replace(
replace(
replace(
replace(
replace(
replace(
replace(
replace(
regexp_replace(
replace(
replace(
replace(
replace(
replace(
replace(
replace(
replace(
replace(
replace(
view_definition::text,
-- This conversion to json is heavily optimized for performance.
-- The general idea is to use as few regexp_replace() calls as possible.
-- Simple replace() is a lot faster, so we jump through some hoops
-- to be able to use regexp_replace() only once.
-- This has been tested against a huge schema with 250+ different views.
-- The unit tests do NOT reflect all possible inputs. Be careful when changing this!
-- -----------------------------------------------
-- pattern | replacement | flags
-- -----------------------------------------------
-- `,` is not part of the pg_node_tree format, but used in the regex.
-- This removes all `,` that might be part of column names.
',' , ''
-- The same applies for `{` and `}`, although those are used a lot in pg_node_tree.
-- We remove the escaped ones, which might be part of column names again.
), E'\\{' , ''
), E'\\}' , ''
-- The fields we need are formatted as json manually to protect them from the regex.
), ' :targetList ' , ',"targetList":'
), ' :resno ' , ',"resno":'
), ' :resorigtbl ' , ',"resorigtbl":'
), ' :resorigcol ' , ',"resorigcol":'
-- Make the regex also match the node type, e.g. `{QUERY ...`, to remove it in one pass.
), '{' , '{ :'
-- Protect node lists, which start with `({` or `((` from the greedy regex.
-- The extra `{` is removed again later.
), '((' , '{(('
), '({' , '{({'
-- This regex removes all unused fields to avoid the need to format all of them correctly.
-- This leads to a smaller json result as well.
-- Removal stops at `,` for used fields (see above) and `}` for the end of the current node.
-- Nesting can't be parsed correctly with a regex, so we stop at `{` as well and
-- add an empty key for the followig node.
), ' :[^}{,]+' , ',"":' , 'g'
-- For performance, the regex also added those empty keys when hitting a `,` or `}`.
-- Those are removed next.
), ',"":}' , '}'
), ',"":,' , ','
-- This reverses the "node list protection" from above.
), '{(' , '('
-- Every key above has been added with a `,` so far. The first key in an object doesn't need it.
), '{,' , '{'
-- pg_node_tree has `()` around lists, but JSON uses `[]`
), '(' , '['
), ')' , ']'
-- pg_node_tree has ` ` between list items, but JSON uses `,`
), ' ' , ','
-- `<>` in pg_node_tree is the same as `null` in JSON, but due to very poor performance of json_typeof
-- we need to make this an empty array here to prevent json_array_elements from throwing an error
-- when the targetList is null.
), '<>' , '[]'
)::json as view_definition
from views
),
target_entries as(
select
view_id, view_schema, view_name,
json_array_elements(view_definition->0->'targetList') as entry
from transform_json
),
results as(
select
view_id, view_schema, view_name,
(entry->>'resno')::int as view_column,
(entry->>'resorigtbl')::oid as resorigtbl,
(entry->>'resorigcol')::int as resorigcol
from target_entries
),
recursion as(
select r.*
from results r
where view_schema = ANY ($1)
union all
select
view.view_id,
view.view_schema,
view.view_name,
view.view_column,
tab.resorigtbl,
tab.resorigcol
from recursion view
join results tab on view.resorigtbl=tab.view_id and view.resorigcol=tab.view_column
)
select
sch.nspname as table_schema,
tbl.relname as table_name,
col.attname as table_column_name,
rec.view_schema,
rec.view_name,
vcol.attname as view_column_name
from recursion rec
join pg_class tbl on tbl.oid = rec.resorigtbl
join pg_attribute col on col.attrelid = tbl.oid and col.attnum = rec.resorigcol
join pg_attribute vcol on vcol.attrelid = rec.view_id and vcol.attnum = rec.view_column
join pg_namespace sch on sch.oid = tbl.relnamespace
join pks_fks using (resorigtbl, resorigcol)
order by view_schema, view_name, view_column_name;
|
|
@steve-chavez , Thanks. Actually I am having 10 schemas in database and got this error while running the above query with one particular schema. Later I temporarily removed that schema only from postgREST and was able to start the service properly. What may be the issue with that schema? |
Ah, that's very interesting. Are you sure you are running If it turns out to be unrelated to |
Ah, I see you already answered that above:
Let's track this in a new issue, as it's very likely to be independent of this. |
@dgtrapeze Just wanted to reaffirm this in case there's any doubt. The Also when testing with Btw, I'm removing the |
|
Great. I'm using v8.0.0.20211102 without issue (although we got caught out by the change in #1849 which changed the response structure from array to single object when we updated from V7 to V8), but haven't noticed any other issues due to standard_conforming_strings |
Environment
Description of issue
I just download postgrest 8.0.0 to try it out. We previously used postgrest 7.0.1. However, on starting up postgrest it just continually output
The pg logs show the query being executed as the pfkSourceColumns query.
Our application started life on postgresql 7 and has upgraded along the way. However, as it predates
standard_conforming_strings, we need to runstandard_conforming_strings=offfor our application to run as lots of queries assume the old escape system. This is set in postgresql.conf.The above query assumes
standard_conforming_strings=onand since we have it off, the query fails to work as expected and the resulting string which is cast to json is not correctly formatted.To get postgrest to work again, I've had to do
where web_authenticator is our role that postgrest uses to connect.
I suspect postgrest assumes standard conforming strings in other places as well now (eg escaping in query parameters was added in v8 I saw?).
I'm not yet fully sure of the impacts to our application having set this for the postgrest authenticator role. We are usually pretty good at using E'' strings now, but you never know what developers do....
I would assume the fix is for postgrest to use escape string syntax (E'' strings) to insulate it from these sorts of database settings.
It could automatically set
standard_conforming_strings=onfor its session (like it does for search_path) or you could add docs to do the alter role authenticator I had to do, but I suspect those options have side-effects.(Expected behavior vs actual behavior)
I expected postgrest 8 to pretty much work the same as postgrest 7.
(Steps to reproduce: Include a minimal SQL definition plus how you make the request to PostgREST and the response body)
then start postgrest
The text was updated successfully, but these errors were encountered: