Skip to content

Make it possible to use copy_from/copy_to with dynamic tables #4549

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make it possible to use copy_from/copy_to with dynamic tables #4549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions diesel/src/pg/query_builder/copy/copy_from.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::borrow::Cow;
use std::marker::PhantomData;
use std::rc::Rc;

use byteorder::NetworkEndian;
use byteorder::WriteBytesExt;
Expand Down Expand Up @@ -87,7 +88,7 @@ impl CopyFromOptions {
pub struct CopyFrom<S, F> {
options: CopyFromOptions,
copy_callback: F,
p: PhantomData<S>,
target: S,
}

pub(crate) struct InternalCopyFromQuery<S, T> {
Expand Down Expand Up @@ -163,7 +164,7 @@ where
&'b self,
pass: crate::query_builder::AstPass<'_, 'b, Pg>,
) -> crate::QueryResult<()> {
S::walk_target(pass)
self.target.walk_target(pass)
}
}

Expand Down Expand Up @@ -269,12 +270,16 @@ macro_rules! impl_copy_from_insertable_helper_for_values_clause {
diesel_derives::__diesel_for_each_tuple!(impl_copy_from_insertable_helper_for_values_clause);

#[derive(Debug)]
pub struct InsertableWrapper<I>(Option<I>);
pub struct InsertableWrapper<I, T> {
table: Rc<T>,
insertable: Option<I>,
}

impl<I, T, V, QId, const STATIC_QUERY_ID: bool> CopyFromExpression<T> for InsertableWrapper<I>
impl<I, T, V, QId, const STATIC_QUERY_ID: bool> CopyFromExpression<T> for InsertableWrapper<I, T>
where
I: Insertable<T, Values = BatchInsert<Vec<V>, T, QId, STATIC_QUERY_ID>>,
V: CopyFromInsertableHelper,
T: CopyTarget,
{
type Error = crate::result::Error;

Expand All @@ -298,7 +303,7 @@ where
// this skips reallocating
let mut buffer = Vec::<u8>::new();
let values = self
.0
.insertable
.take()
.expect("We only call this callback once")
.values();
Expand Down Expand Up @@ -354,7 +359,7 @@ where
&'b self,
pass: crate::query_builder::AstPass<'_, 'b, Pg>,
) -> crate::QueryResult<()> {
<V as CopyFromInsertableHelper>::Target::walk_target(pass)
self.table.walk_target(pass)
}
}

Expand All @@ -372,7 +377,7 @@ where
#[must_use = "`COPY FROM` statements are only executed when calling `.execute()`."]
#[cfg(feature = "postgres_backend")]
pub struct CopyFromQuery<T, Action> {
table: T,
table: Rc<T>,
Copy link
Member

Choose a reason for hiding this comment

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

Using an Rc here is unfortunately a breaking change as this turns this struct from something that implements Send into something that doesn't implement Send.

Fortunately we don't need to explicitly store table inside of InsertableWrapper. Instead we can just pass a reference in through the method call chain.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I'll incorporate it, but it might take me a little bit to get around to it

action: Action,
}

Expand All @@ -386,15 +391,15 @@ where
/// `action` expects a callback which accepts a [`std::io::Write`] argument. The necessary format
/// accepted by this writer sink depends on the options provided via the `with_*` methods
#[allow(clippy::wrong_self_convention)] // the sql struct is named that way
pub fn from_raw_data<F, C, E>(self, _target: C, action: F) -> CopyFromQuery<T, CopyFrom<C, F>>
pub fn from_raw_data<F, C, E>(self, target: C, action: F) -> CopyFromQuery<T, CopyFrom<C, F>>
where
C: CopyTarget<Table = T>,
F: Fn(&mut dyn std::io::Write) -> Result<(), E>,
{
CopyFromQuery {
table: self.table,
action: CopyFrom {
p: PhantomData,
target,
options: Default::default(),
copy_callback: action,
},
Expand All @@ -411,13 +416,16 @@ where
/// This uses the binary format. It internally configures the correct
/// set of settings and does not allow to set other options
#[allow(clippy::wrong_self_convention)] // the sql struct is named that way
pub fn from_insertable<I>(self, insertable: I) -> CopyFromQuery<T, InsertableWrapper<I>>
pub fn from_insertable<I>(self, insertable: I) -> CopyFromQuery<T, InsertableWrapper<I, T>>
where
InsertableWrapper<I>: CopyFromExpression<T>,
InsertableWrapper<I, T>: CopyFromExpression<T>,
{
CopyFromQuery {
table: self.table,
action: InsertableWrapper(Some(insertable)),
table: self.table.clone(),
action: InsertableWrapper {
table: self.table,
insertable: Some(insertable),
},
}
}
}
Expand Down Expand Up @@ -650,7 +658,7 @@ where
T: Table,
{
CopyFromQuery {
table,
table: Rc::new(table),
action: NotSet,
}
}
9 changes: 4 additions & 5 deletions diesel/src/pg/query_builder/copy/copy_to.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::io::BufRead;
use std::marker::PhantomData;

use super::CommonOptions;
use super::CopyFormat;
Expand Down Expand Up @@ -57,7 +56,7 @@ impl QueryFragment<Pg> for CopyToOptions {
#[derive(Debug)]
pub struct CopyToCommand<S> {
options: CopyToOptions,
p: PhantomData<S>,
target: S,
}

impl<S> QueryId for CopyToCommand<S>
Expand All @@ -79,7 +78,7 @@ where
) -> crate::QueryResult<()> {
pass.unsafe_to_cache_prepared();
pass.push_sql("COPY ");
S::walk_target(pass.reborrow())?;
self.target.walk_target(pass.reborrow())?;
pass.push_sql(" TO STDOUT");
self.options.walk_ast(pass.reborrow())?;
Ok(())
Expand Down Expand Up @@ -301,7 +300,7 @@ where
let io_result_mapper = |e| crate::result::Error::DeserializationError(Box::new(e));

let command = CopyToCommand {
p: PhantomData::<U::SelectExpression>,
target: self.target,
options: CopyToOptions {
header: None,
common: CommonOptions {
Expand Down Expand Up @@ -410,7 +409,7 @@ where
{
let q = O::setup_options(self);
let command = CopyToCommand {
p: PhantomData::<T>,
target: q.target,
options: q.options,
};
ExecuteCopyToConnection::execute(conn, command)
Expand Down
12 changes: 9 additions & 3 deletions diesel/src/pg/query_builder/copy/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@ pub trait CopyTarget {
type SqlType: SqlType;

#[doc(hidden)]
fn walk_target(pass: crate::query_builder::AstPass<'_, '_, Pg>) -> crate::QueryResult<()>;
fn walk_target(
&self,
pass: crate::query_builder::AstPass<'_, '_, Pg>,
) -> crate::QueryResult<()>;
}

impl<T> CopyTarget for T
Expand All @@ -130,7 +133,10 @@ where
type Table = Self;
type SqlType = T::SqlType;

fn walk_target(mut pass: crate::query_builder::AstPass<'_, '_, Pg>) -> crate::QueryResult<()> {
fn walk_target(
&self,
mut pass: crate::query_builder::AstPass<'_, '_, Pg>,
) -> crate::QueryResult<()> {
T::STATIC_COMPONENT.walk_ast(pass.reborrow())?;
pass.push_sql("(");
T::all_columns().walk_ast(pass.reborrow())?;
Expand All @@ -157,7 +163,7 @@ macro_rules! copy_target_for_columns {
type Table = T;
type SqlType = crate::dsl::SqlTypeOf<Self>;

fn walk_target(
fn walk_target(&self,
mut pass: crate::query_builder::AstPass<'_, '_, Pg>,
) -> crate::QueryResult<()> {
T::STATIC_COMPONENT.walk_ast(pass.reborrow())?;
Expand Down
Loading