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

insert returning panic when returning implicit padded NULL values #9539

Closed
broccoliSpicy opened this issue Apr 28, 2023 · 7 comments · Fixed by #11080
Closed

insert returning panic when returning implicit padded NULL values #9539

broccoliSpicy opened this issue Apr 28, 2023 · 7 comments · Fixed by #11080
Assignees
Labels
no-issue-activity type/bug Something isn't working
Milestone

Comments

@broccoliSpicy
Copy link
Contributor

broccoliSpicy commented Apr 28, 2023

Describe the bug

perhaps we shouldn't punt the NULL padding logic to the batch executor.

To Reproduce

dev=> create table t (v1 int, v2 real);
CREATE_TABLE
dev=> insert into t (v2) values (1) returning *;
SSL SYSCALL error: EOF detected
The connection to the server was lost. Attempting reset: Failed.

Expected behavior

postgres=# create table t1 (v1 int, v2 real);
CREATE TABLE
postgres=# insert into t1 (v2) values (1) returning *;
 v1 | v2 
----+----
    |  1
(1 row)

INSERT 0 1
postgres=# 

Additional context

No response

@broccoliSpicy broccoliSpicy added the type/bug Something isn't working label Apr 28, 2023
@github-actions github-actions bot added this to the release-0.20 milestone Apr 28, 2023
@broccoliSpicy
Copy link
Contributor Author

broccoliSpicy commented Apr 28, 2023

@BugenZhao @Eurekaaw PTAL

@broccoliSpicy
Copy link
Contributor Author

I think revert the NULL padding punt we did earlier is the easiest way to fix this.

NULL padding punt PR

@y-wei
Copy link
Contributor

y-wei commented Apr 28, 2023

I'd suggest the problem is:

let schema = if returning {
input.schema().clone()
} else {

The schema shall be the source's schema plus padding columns. Afterwards in insert executor a datachunk shall be yielded after evaluating default columns;

postgres=# create table td (v1 int default 1, v2 int);
CREATE TABLE
postgres=# insert into td (v2) values (0) returning *;
 v1 | v2 
----+----
  1 |  0
(1 row)

@broccoliSpicy broccoliSpicy changed the title insert returning panic when returning implicited padded NULL values insert returning panic when returning implicit padded NULL values Apr 28, 2023
@broccoliSpicy
Copy link
Contributor Author

broccoliSpicy commented Apr 29, 2023

just out of curiosity, do we have a design to handle create table td (v1 int default 1, v2 int); ?

in the case of create table t( a int, b timestamp default current_timestamp);, we may want to evaluate the function as soon as the prompt arrives, I suppose?
so we need to handle it in the frontend?
in a distributed system like risingwave, there might be some inconsistency if we evaluate the function current_timestamp in create table t( a int, b timestamp default current_timestamp) in the batch executor.

@y-wei
Copy link
Contributor

y-wei commented Apr 29, 2023

just out of curiosity, do we have a design to handle create table td (v1 int default 1, v2 int); ?

Yes! See #8999

@broccoliSpicy
Copy link
Contributor Author

Screenshot from 2023-04-29 21-31-27

this is not very kind of you ...

@github-actions
Copy link
Contributor

This issue has been open for 60 days with no activity. Could you please update the status? Feel free to continue discussion or close as not planned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-issue-activity type/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants