Fix bug "UNION types \"char\" and text cannot be matched" with v9.0.1.+++ versions

[yevon]

The query in:

postgrest/src/PostgREST/DbStructure.hs

Lines 655 to 809 in bac63f3

	with recursive
	pks_fks as (
	-- pk + fk referencing col
	select
	contype,
	conname,
	conrelid as resorigtbl,
	unnest(conkey) as resorigcol
	from pg_constraint
	where contype IN ('p', 'f')
	union
	-- fk referenced col
	select
	concat(contype, '_ref') as contype,
	conname,
	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(
	regexp_replace(
	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
	-- -----------------------------------------------
	-- `<>` 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.
	-- We'll need to put it first, to make the node protection below work for node lists that start with
	-- null: `(<> ...`, too. This is the case for coldefexprs, when the first column does not have a default value.
	'<>' , '()'
	-- `,` 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 `,`
	), ' ' , ','
	)::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,
	rec.view_schema,
	rec.view_name,
	pks_fks.conname as constraint_name,
	pks_fks.contype as constraint_type,
	array_agg(row(col.attname, vcol.attname) order by col.attnum) as column_dependencies
	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)
	group by sch.nspname, tbl.relname, rec.view_schema, rec.view_name, pks_fks.conname, pks_fks.contype

Doesn't properly consider the type of the column contype (char) of the table pg_constraint. So when it tries to make a union with the concat function (text), fails with a missmatch type error. This fixes the issue.

Related issue here:

Reported issue

#2410

[steve-chavez]

I don't have a pg 15 to test it, but LGTM!

Could you add an entry to the Fixed section in the CHANGELOG?

[yevon]

I don't have a pg 15 to test it, but LGTM!

Could you add an entry to the Fixed section in the CHANGELOG?

Done!

[wolfgangwalther]

I think this is part of a bigger issue: PG15 support. It makes little sense to me to commit something as isolated as this now.

We need to ensure this doesn't break again and that we don't have any other problems with PG15. So we should add PG15 to our postgres versions in nix. I don't think this version is currently in the nixpkgs, so we'd need to figure out how to install it. But I guess we should do that once, and then we can always use the latest beta version ahead of time to confirm everything still works.

[wolfgangwalther]

I don't think this version is currently in the nixpkgs, so we'd need to figure out how to install it.

Looks like pg14 was added as beta1 already to nixpkgs last year: NixOS/nixpkgs@285fa46

So maybe we just need somebody to put up a PR for pg15 beta 2?

[wolfgangwalther]

So maybe we just need somebody to put up a PR for pg15 beta 2?

That somebody was me, I guess: NixOS/nixpkgs#185751

[yevon]

I haven't checked if this is working with previous versions of postgres. But it seems that it should also fail because this union is wrong anyway in any postgres version. Is it possible that the column contype was added just after stable 9.0.1? 9.0.1 is working nicely with postgres 15 beta 2.

[wolfgangwalther]

I haven't checked if this is working with previous versions of postgres. But it seems that it should also fail because this union is wrong anyway in any postgres version.

Right, that makes sense. Can you test this with pg14, too?

If that fails as well, we need to find out what you have in your schema that triggers this, because we certainly want to test this to avoid a regression.

Is it possible that the column contype was added just after stable 9.0.1? 9.0.1 is working nicely with postgres 15 beta 2.

No, not really. The same construct was there before, but it was moved around to a different query since.

[yevon]

I will test this with postgres 14

[wolfgangwalther]

Is it possible that the column contype was added just after stable 9.0.1? 9.0.1 is working nicely with postgres 15 beta 2.

No, not really. The same construct was there before, but it was moved around to a different query since.

Scratch that, I confused this with something else. The contype was indeed added in the meantime in d271942.

[wolfgangwalther]

I'm pretty sure this issue came up after the change discussed here https://www.postgresql.org/message-id/flat/2216388.1638480141%40sss.pgh.pa.us

So this should really be a pg15 issue only.

[yevon]

You are right then, postgres 15 seems to not consider char as text anymore as it is deprecated and this should not affect past versions of postgres. Anyway it is working fine until postgREST 9.0.1 stable so this is due to recent changes only. Do I have a way to build a docker image locally to try this out? It was trying to find a dockerfile, but it seems that you use Nix. I don't know anything about nix but it should be pretty the same?

[yevon]

Could you build a docker image with this change so I can try this with postgres 15?

[steve-chavez]

Seeing that NixOS/nixpkgs#185751 is blocked, I think we should just merge here.

I've added a note here for testing pg15 when it's available.

[laurenceisla]

I've run the tests run the changes and they work successfully for docker postgres 15 beta2 and beta3.

[yevon]

Suggested syntax change done! Also modified the column alias syntax "as contype" as it seems that this is always used for column alias.

[yevon]

Confirm is working nicely version v10.0.0!

[rrbagautdinov]

Hi, i have similar problem on pg:latest (15) tag (docker)

[yevon]

Hi, i have similar problem on pg:latest (15) tag (docker)

Upgrade postgrest to at least v10.0.0, this is not happening anymore, already solved.