Bug: foreign call that returns tables doesn't correctly check value and names

[outerlook]

Version / Commit

main branch (327a34f)

Operating System

Ubuntu 22.04.4 LTS

Configuration

None

Expected Behavior

foreign procedure ext_get_record($date_from text, $date_to text, $frozen_at text) returns table(
    date_value text,
    value int
)

should work, I guess (or is there a syntax error?)

Actual Behavior

procedure get_unsafe_record($dbid text, $date_from text, $date_to text, $frozen_at text) public view returns table(
    date_value text,
    value int
) {
    for $row in SELECT * FROM ext_get_record[$dbid, 'get_record']($date_from, $date_to, $frozen_at) {
        return next $row.date_value, $row.value;
    }
}

Produces error

ERROR: Foreign procedure definition "ext_get_record" expects return name "date_value" at column position 1, but procedure "get_record" requires text (SQLSTATE P0001)

Steps to Reproduce

• deploy contract with a foreign function that returns tables
• try getting dataa

Additional Information

while searching for the cause, I found that this was the generated code for the foreign call:

create function _fp_ext_get_record(_dbid text, _procedure text, _arg1 text, _arg2 text, _arg3 text) returns TABLE(date_value text, value bigint)
	language plpgsql
as $$ DECLARE
    _schema_owner BYTEA;
    _is_view BOOLEAN;
    _is_owner_only BOOLEAN;
    _is_public BOOLEAN;
	_returns_table BOOLEAN;
    _expected_input_types TEXT[];
    _expected_return_types TEXT[];
BEGIN
	SELECT p.param_types, p.return_types, p.is_view, p.owner_only, p.public, s.owner, p.returns_table
	INTO _expected_input_types, _expected_return_types, _is_view, _is_owner_only, _is_public, _schema_owner, _returns_table
	FROM kwild_internal.procedures as p INNER JOIN kwild_internal.kwil_schemas as s
	ON p.schema_id = s.id
	WHERE p.name = _procedure AND s.dbid = _dbid;

	IF _schema_owner IS NULL THEN
		RAISE EXCEPTION 'Procedure "%" not found in schema "%"', _procedure, _dbid;
	END IF;

	IF _is_view = FALSE AND current_setting('is_read_only') = 'on' THEN
		RAISE EXCEPTION 'Non-view procedure "%" called in view-only connection', _procedure;
	END IF;

	IF _is_owner_only = TRUE AND _schema_owner != current_setting('ctx.signer')::BYTEA THEN
		RAISE EXCEPTION 'Procedure "%" is owner-only and cannot be called by signer "%" in schema "%"', _procedure, current_setting('ctx.signer'), _dbid;
	END IF;

	IF _is_public = FALSE THEN
        RAISE EXCEPTION 'Procedure "%" is not public and cannot be foreign called', _procedure;
    END IF;
	
	IF array_length(_expected_input_types, 1) IS NULL THEN
		RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects 3 inputs, but procedure "%" requires no inputs', _procedure;
	END IF;

	IF array_length(_expected_input_types, 1) != 3 THEN
		RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects 3 inputs, but procedure "%" requires % inputs', _procedure, array_length(_expected_input_types, 1);
	END IF;
	IF _expected_input_types[1] != 'text' THEN
		RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects input type "text", but procedure "%" requires %', _procedure, _expected_input_types[1];
	END IF;
	IF _expected_input_types[2] != 'text' THEN
		RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects input type "text", but procedure "%" requires %', _procedure, _expected_input_types[2];
	END IF;
	IF _expected_input_types[3] != 'text' THEN
		RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects input type "text", but procedure "%" requires %', _procedure, _expected_input_types[3];
	END IF;
		IF _returns_table = FALSE THEN
			RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects a table return, but procedure "%" does not return a table', _procedure;
		END IF;
			IF _expected_return_types[1] != 'text' THEN
				RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects return type "text" at column position 1, but procedure "%" requires %', _procedure, _expected_return_types[1];
			END IF;
			IF _expected_return_types[1] != 'date_value' THEN
				RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects return name "date_value" at column position 1, but procedure "%" requires %', _procedure, _expected_return_types[1];
			END IF;
			IF _expected_return_types[2] != 'int' THEN
				RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects return type "int" at column position 2, but procedure "%" requires %', _procedure, _expected_return_types[2];
			END IF;
			IF _expected_return_types[2] != 'value' THEN
				RAISE EXCEPTION 'Foreign procedure definition "ext_get_record" expects return name "value" at column position 2, but procedure "%" requires %', _procedure, _expected_return_types[2];
			END IF;
	RETURN QUERY EXECUTE format('SELECT * FROM ds_%I.%I($1, $2, $3)', _dbid, _procedure) USING _arg1, _arg2, _arg3; END; $$;

alter function _fp_ext_get_record(text, text, text, text, text) owner to kwild;

The error is more specific to these lines:

...
			IF _expected_return_types[1] != 'text' THEN
...
			IF _expected_return_types[1] != 'date_value' THEN
...
			IF _expected_return_types[2] != 'int' THEN
...
			IF _expected_return_types[2] != 'value' THEN
...

they all refer to types, while I think it should refer to types and then names after them, correct?

I know that GH issues may not be the best to notify, so I'll ping you directly (please say if you prefer not to) @KwilLuke @brennanjl

[brennanjl]

Resolved in: #771

[brennanjl]

Also, thank you a ton for flagging this 🙏. Made it very easy to debug, and uncovered a few other edge cases in our procedure tests.