Skip to content
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

cannot accept type record #39

Open
mtvan opened this issue May 15, 2022 · 18 comments
Open

cannot accept type record #39

mtvan opened this issue May 15, 2022 · 18 comments

Comments

@mtvan
Copy link

mtvan commented May 15, 2022

select pg_background_result(PID);
consistently returns the following error

Query 1 ERROR: ERROR: function returning record called in context that cannot accept type record HINT: Try calling the function in the FROM clause using a column definition list.

even though the pg_background_launch(SQL) ran successfully and I can confirm the query inside it ran fine. I'm on Postgres 14.2

@vibhorkum
Copy link
Owner

I tried it in my environment. However, I cannot reproduce it.
Can you please share a test case to reproduce this?

Thank you!

@kirkw
Copy link

kirkw commented Jul 25, 2022

Okay, I totally understand this error. I am using Ubuntu, PG 14.4

The root of these problems are that the pg_background_result() is attempting to MATCH the result (of the passed in SQL) into a record set and return the rows. COOL. And very dynamic.

So, this works:
select * from pg_background_result( pg_background_launch($$select pg_sleep(3); select 'done';$$) ) as (result text);

-- but, removing the "as (result text)" throws an error:
ERROR: a column definition list is required for functions returning "record"

reducing it to a simpler select (with no record specifier, and no from, as @mtvan did):
select pg_background_result( pg_background_launch($$select pg_sleep(3); select 'done';$$) );
ERROR: function returning record called in context that cannot accept type record
HINT: Try calling the function in the FROM clause using a column definition list.

Reminder, this happens AFTER the query finishes, during the results collection (I had to read the C code, LOL).

TODO: Please CONSIDER creating a pg_background_wait(); -- That ignores the result, and just waits for it to finish...

Continuing with this example: (because I NOW have 2 fields coming back, result text does NOT work)
select * from pg_background_result( pg_background_launch($$select pg_sleep(3); select 'done','here';$$) ) as (result text);
ERROR: remote query result rowtype does not match the specified FROM clause rowtype
STATEMENT: select * from pg_background_result( pg_background_launch($$select pg_sleep(3); select 'done','here';$$) ) as (result text);

-- Now, if I understand the problem, I should be able to get as (Result text, Other Text) to work! [Basically MATCHING the result set of the query being executed], and it DOES work!

select * from pg_background_result( pg_background_launch($$select pg_sleep(3); select 'done','here';$$) ) as (Result text, Other text);
result | other
--------+-------
done | here

THANK YOU for addressing this, and supporting this.
I hope these details help to explain the problem @mtvan was having (and so am I).
I also think the README could use a couple of examples like this so people understand how to code it up...
Use these if you like.. (I love the pg_sleep() plus something else because you know it is waiting!)

@kirkw
Copy link

kirkw commented Aug 30, 2022

@vibhorkum Any feedback on this? I Felt the detail I added was clarifying as to the cause of the problem.
I keep running into this in my code, as I've Wrapped the code. I use it for autonomous transactions.
And I don't normally care about the return values, and MANY calls are INSERTS() for logging reasons (99%).
I keep adding: "; select now()::text as result; " to force success...

@kirkw
Copy link

kirkw commented Oct 5, 2022

@vibhorkum Curious if you have seen my additional examples. Any Comments?

@rjuju
Copy link
Collaborator

rjuju commented Oct 5, 2022

I see 2 ways to address your problem:

  • add a new argument to pg_background_result, something like "bool emit_tuples" defaulting to true, and if you pass false it just ignores any tuple descriptor mismatch and all rows. Note that in that case the function would currently switch to emitting the command tags of all processed queries, so you may see a bunch of "INSERT 0 1" or similar depending on what your query text said. It also therefore requires to specify a single output column of type text. That may be nice to validate that it seemed to have done what you asked for, but may be noisy
  • add a new function that also consumes all the input (this is required in order to retrieve all possible messages emitted during the query text execution), but discard all received rows and doesn't output the command tags (or does it optionally with some parameter, it wouldn't be that much of a problem here as you wouldn't have to specify the tuple description)

Would you be interested in any of those?

@kirkw
Copy link

kirkw commented Oct 5, 2022

@rjuju I would prefer the second option... pg_background_lossy_result() that ALWAYS returns only the simple text rows. I like the name LOSSY in there because it makes you "think" and read the docs, that you are giving up any complex output, but allowing it to be processed, but not returned (hence it is lossy). But you call it what you want.

I also believe updating the README with an example that returns multiple columns, and the error if you trap it wrong, as I worked through, might be helpful to others? (Feel free to use any of my text, Feel Free to ask me to give it a shot, and do a Pull Request for it. Here to help!)

Much Appreciated!

@rjuju
Copy link
Collaborator

rjuju commented Oct 5, 2022

I'm not sure what you mean by the simple text rows. Is it the command tags, ie. something like that result ? (quick local hack):

=# SELECT * FROM pg_background_result(pg_background_launch($$select 'toto';drop table if exists sometable;select 1; insert into tt select 1; select 'a', 'b' from generate_series(1, 10000)$$)) as (tag text);
NOTICE:  00000: table "sometable" does not exist, skipping
     tag
--------------
 SELECT 1
 DROP TABLE
 SELECT 1
 INSERT 0 1
 SELECT 10000
(5 rows)

@rjuju
Copy link
Collaborator

rjuju commented Oct 5, 2022

I also believe updating the README with an example that returns multiple columns, and the error if you trap it wrong, as I worked through, might be helpful to others? (Feel free to use any of my text, Feel Free to ask me to give it a shot, and do a Pull Request for it. Here to help!)

I think I already saw multiple people facing such problems, so I definitely agree it would be great. Maybe adding a Frequently Asked Question section on the readme would be enough (covering the lack of FROM and the tuple description mismatch)? If you're willing to send a PR I will gladly review it!

@kirkw
Copy link

kirkw commented Oct 5, 2022

as (tag text);

is EXACTLY what I meant by a simple "simple text rows". Each row is just one TEXT column.
The crux of the problem.

Okay, give me a day or so to allocate some time to the README!

My hack has been to add an EXTRA piece which guarantees only one column comes back, since we only RETURN the last result set;

I have QUIET on... So I don't get the SELECT / INSERT messages. But curious that I don't see the select 10::text results.
I assume that is because we really only send back the LAST Query Results. (which makes me wonder if I had 65k of other query results first... But I think they would be just ignored, and not require any further processing...)

select * from pg_background_result(pg_background_launch('select 10::text; select 1,2,3; select now()::text as result;')) as (result text);
            result
-------------------------------
 2022-10-05 10:30:57.507178-04

And removing that last select, throws the error in question (for me):

select * from pg_background_result(pg_background_launch('select 10::text; select 1,2,3;')) as (result text);
ERROR:  remote query result rowtype does not match the specified FROM clause rowtype
STATEMENT:  select * from pg_background_result(pg_background_launch('select 10::text; select 1,2,3;')) as (result text);

@rjuju
Copy link
Collaborator

rjuju commented Oct 5, 2022

is EXACTLY what I meant by a simple "simple text rows". Each row is just one TEXT column.
The crux of the problem.

I understand your root problem, but I was meaning a case where it's guaranteed that you need a single text column since it's ignoring the normal output. The question is whether you want to get the command tags or not, when ignoring the rest of the rows, and whether you want a dedicated function (which would avoid typing the column definition).

So either:

=# SELECT * FROM pg_background_result(pg_background_launch($$select 'toto';
drop table if exists sometable;select 1;
insert into tt select 1;
select 'a', 'b' from generate_series(1, 10000)$$), false) as (tag text);
NOTICE:  00000: table "sometable" does not exist, skipping
     tag
--------------
 SELECT 1
 DROP TABLE
 SELECT 1
 INSERT 0 1
 SELECT 10000
(5 rows)

or

=# SELECT * FROM pg_background_discard_result(pg_background_launch($$select 'toto';
drop table if exists sometable;select 1; insert into tt select 1;
select 'a', 'b' from generate_series(1, 10000)$$));
NOTICE:  00000: table "sometable" does not exist, skipping
     tag
--------------
 SELECT 1
 DROP TABLE
 SELECT 1
 INSERT 0 1
 SELECT 10000
(5 rows)

or

=# SELECT * FROM pg_background_discard_result(pg_background_launch($$select 'toto';
drop table if exists sometable;select 1; insert into tt select 1;
select 'a', 'b' from generate_series(1, 10000)$$));
NOTICE:  00000: table "sometable" does not exist, skipping
 pg_background_discard_result
---------------------------------

(1 row)

@kirkw
Copy link

kirkw commented Oct 5, 2022

Sorry, I misunderstood the root of the question. I am perfectly fine with the LAST example.
I do NOT care about the result at all...

This call is wrapped in plpgsql via a PERFORM as it is...
So, the result is ignored...

I feel if the result is important to you... Then you should understand it, and know how to retrieve it (Proper record type)

@rjuju
Copy link
Collaborator

rjuju commented Oct 5, 2022

That makes sense.

I have a preliminary patch that I pushed at 602787d on a "discard_rows" branch. If you can compile the extension, can you confirm that it's doing what you want?

@kirkw
Copy link

kirkw commented Oct 6, 2022

Okay, I built it:
make clean
make
make install

I did a restart on postgresql...

But I do not see the function?
I tried ALTER EXTENSION pg_background UPDATE; -- Nothing

Do I have to create the SQL to declare the function? If so, can you point it out? (is it similar to pg_background_launch)

Thanks!

@rjuju
Copy link
Collaborator

rjuju commented Oct 6, 2022

Unless you have pg_background in shared_preload_libraries (which really you shouldn't), there's no need to restart. A new connection should load the new code.

I tried ALTER EXTENSION pg_background UPDATE; -- Nothing

That's weird, are you sure that you installed the extension for the correct major version? Is pg_config yielding info for the correct instance?

Does this query show the version 1.1 available?

select name, version from pg_available_extension_versions where name = 'pg_background';

@kirkw
Copy link

kirkw commented Oct 6, 2022

select name, version from pg_available_extension_versions where name = 'pg_background';

1.0

But NONE of the SQL files were updated? (Shouldn't there be a new 1.1 SQL file?)

git status:

git status

On branch discard_rows
Untracked files:
(use "git add ..." to include in what will be committed)
pg_background.bc
pg_background.o
pg_background.so

nothing added to commit but untracked files present (use "git add" to track)

@rjuju
Copy link
Collaborator

rjuju commented Oct 6, 2022

I see. So it looks like you're working on the "master" branch. Do you see the pg_background--1.1.sql locally? If not, can you do git checkout discard_rows first. You should then see the new files, and a make clean / make / make install cycle should make it available on postgres.

@kirkw
Copy link

kirkw commented Oct 6, 2022

okay, I see it now...
(I might have typoed the switch on the repo (leaving out origin/)) Again, new to this.

I just tested this. And YES it works as I was hoping!

I am curious about ONE thing. How do the NOTICE messages KNOW to come to my terminal? (I was not expecting that, but it is PERFECTLY FINE)...

@rjuju
Copy link
Collaborator

rjuju commented Oct 6, 2022

I just tested this. And YES it works as I was hoping!

Great news! I will double check that there's no problem with the code, cleanup the comments and push it shortly.

I am curious about ONE thing. How do the NOTICE messages KNOW to come to my terminal? (I was not expecting that, but it is PERFECTLY FINE)...

That's because pg_background uses the same infrastructure as parallel queries, which will report any error data (NOTICE, ERROR or anything else) to the consumer of the main pid even if the message is emitted by a parallel worker.

So the query itself just emit the message, which is automatically redirected to the message queue by pg_background, and the pg_background_result (and pg_background_discard_result) receive the message and rethrow it to you. That's why you still need to consume all the incoming protocol messages, even if you want to see most of them, if you want to be notified of any problem happening during the query execution in the background.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants