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

PG16 - Add COPY FROM default tests #7143

Merged
merged 2 commits into from
Aug 24, 2023
Merged

Conversation

naisila
Copy link
Member

@naisila naisila commented Aug 24, 2023

Already supported in Citus, adding the same tests as in PG
Relevant PG commit:
postgres/postgres@9f8377f

#7138

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #7143 (c7dae63) into main (70c8aba) will decrease coverage by 0.10%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #7143      +/-   ##
==========================================
- Coverage   93.41%   93.32%   -0.10%     
==========================================
  Files         272      272              
  Lines       58990    58988       -2     
==========================================
- Hits        55108    55048      -60     
- Misses       3882     3940      +58     

-- COPY FROM ... DEFAULT
-- Already supported in Citus, adding all PG tests with a distributed table
-- Relevant PG commit:
-- https://github.com/postgres/postgres/commit/9f8377f
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy on the shards that are local goes through a different codepath. So, I think it'd be very useful to add one very basic test to make sure it works.

A good alternative could be to create a citus-local (or a reference table), where the shard will be local to the coodinator.

Or, connect to one of the workers and ensure the copy is happening at least one local shard.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, connect to one of the workers and ensure the copy is happening at least one local shard.

Thanks, I picked this one.

@naisila naisila force-pushed the naisila/pg16_copy_from_default branch 2 times, most recently from 0214e91 to a94a63a Compare August 24, 2023 11:59
\c - - - :master_port
TRUNCATE pg16.copy_default;
\c - - - :worker_2_port
COPY pg16.copy_default FROM stdin WITH (format csv, default '\D');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we sure that there are local shards here? if not, please consider adjusting the "id".

I'd be even fine to set citus.log_local_commands TO ON;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are sure one of the two workers has shards, so I just connected to both of them.

Relevant PG commit:
postgres/postgres@9f8377f
Already supported in Citus, adding the same tests as in PG
@naisila naisila force-pushed the naisila/pg16_copy_from_default branch from a94a63a to c7dae63 Compare August 24, 2023 12:36
@naisila naisila merged commit afab879 into main Aug 24, 2023
37 checks passed
@naisila naisila deleted the naisila/pg16_copy_from_default branch August 24, 2023 12:52
francisjodi added a commit to francisjodi/citus that referenced this pull request Aug 30, 2023
Already supported in Citus, adding the same tests as in PG
Relevant PG commit:
postgres/postgres@9f8377f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants