MIME-Version: 1.0
References: <1197371508.4255.1327.camel@ebony.site>
 <1197660423.1536.8.camel@dell.linuxdev.us.dell.com>
 <15007.1197674561@sss.pgh.pa.us> <200712141830.16877.trent_shipley@qwest.net>
 <87k31ftoe0.fsf_-_@commandprompt.com>
 <CAFj8pRC3ksKhaSxDUo3Y1DekvV12OcEERryT8gPGdUAVEe2SLg@mail.gmail.com>
 <CAFj8pRD9Pi9besMvv0Yo0A4wqQ-D=cBXGRQhz_oS+BNgv_41_Q@mail.gmail.com>
 <CALH1LgsKDk4XthrM4m8tAqfoPyNgDCSFV3Go1nevXze-8dwKrA@mail.gmail.com>
 <CAFj8pRC96C+C6Gg8OHkv26YZMQsrRJ9YtJhBKrrrje1hc2yrcw@mail.gmail.com>
 <CALH1LgvmNcnO8dYyckcEmiJ6PGDnpRWA3V1td1SDKzqt6FrMJw@mail.gmail.com>
 <e78a86704ab2b2b967f0d674e5a72643@oss.nttdata.com>
 <CALH1Lgu0dqsQUm9QYdoSvgu-O9Ac5OsbE3TxxZqChUrVF+Qrjw@mail.gmail.com>
 <dc876e926f284ee60eef4ece755fa419@oss.nttdata.com>
 <CALH1Lgv5QeYSWYDgMBd=wvKum8egiaMR-_77VGsiHPt7E8dK9Q@mail.gmail.com>
 <CALH1LgvzpORM+EHVXRz4-Dv+_aHvm9m9Hu6eYM4PfppwKgXvFQ@mail.gmail.com>
 <686e46d6718f9535bc39fac7cad7408b@oss.nttdata.com>
 <CALH1Lgsr7Asz6r6jnRta7DVEjKwyz=b3XJhyCMoYv8Yy9y4cXg@mail.gmail.com>
 <769423519e16d18a1e7611c6931da58b@oss.nttdata.com>
 <CALH1LguQ5P0uZSYQFy4gJihvV11EkLx+gwxZ-+1DFq1Z6dJqbg@mail.gmail.com>
 <CALH1Lgug+mR9A1bBzR8hrp2oW4=jvR_N0An-i2X_j_y9C1fkiA@mail.gmail.com>
 <CALH1LguPAN=p5wHKwGcHrjE8Vj1x4o1z3Pk228YDif6LFzbeYQ@mail.gmail.com>
In-Reply-To: 
 <CALH1LguPAN=p5wHKwGcHrjE8Vj1x4o1z3Pk228YDif6LFzbeYQ@mail.gmail.com>
From: Danil Anisimow <anisimow.d@gmail.com>
Date: Fri, 9 Dec 2022 21:25:34 +0700
Message-ID: 
 <CABm2Ma6_=qf7kZPyOmiK1CUXeio7q08HNPYXrqkCwF3P_KQu7A@mail.gmail.com>
Subject: Re: POC PATCH: copy from ... exceptions to: (was Re: VLDB Features)
To: Damir Belyalov <dam.bel07@gmail.com>
Cc: torikoshia <torikoshia@oss.nttdata.com>,
	"a.lepikhov@postgrespro.ru" <a.lepikhov@postgrespro.ru>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000e2e18105ef65ed8d"
Archived-At: 
 <https://www.postgresql.org/message-id/CABm2Ma6_%3Dqf7kZPyOmiK1CUXeio7q08HNPYXrqkCwF3P_KQu7A%40mail.gmail.com>
Precedence: bulk

--000000000000e2e18105ef65ed8d
Content-Type: text/plain; charset="UTF-8"

Hi!

I have looked at your patch and have a few questions.

110: static bool SafeCopying(CopyFromState cstate, ExprContext *econtext,
111: TupleTableSlot *myslot);
---
636: bool
637: SafeCopying(CopyFromState cstate, ExprContext *econtext,
TupleTableSlot *myslot)

Why is there no static keyword in the definition of the SafeCopying()
function, but it is in the function declaration.

675: MemoryContext cxt =
MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
676:
677: valid_row = NextCopyFrom(cstate, econtext, myslot->tts_values,
myslot->tts_isnull);
678: tuple_is_valid = valid_row;
679:
680: if (valid_row)
681: sfcstate->safeBufferBytes += cstate->line_buf.len;
682:
683: CurrentMemoryContext = cxt;

Why are you using a direct assignment to CurrentMemoryContext instead of
using the MemoryContextSwitchTo function in the SafeCopying() routine?

1160: /* Standard copying with option "safe copying" enabled by
IGNORE_ERRORS. */
1161: if (!SafeCopying(cstate, econtext, myslot))
1162: break;

I checked with GDB that the CurrentMemoryContext changes when SafeCopying
returns. And the target context may be different each time you do a COPY in
psql.

1879: cstate->sfcstate->safe_cxt = AllocSetContextCreate(oldcontext,
1880: "Safe_context",
1881: ALLOCSET_DEFAULT_SIZES);

When you initialize the cstate->sfcstate structure, you create a
cstate->sfcstate->safe_cxt memory context that inherits from oldcontext.
Was it intended to use cstate->copycontext as the parent context here?

On Wed, Nov 2, 2022 at 11:46 AM Damir Belyalov <dam.bel07@gmail.com> wrote:

> Updated the patch:
> - Optimized and simplified logic of IGNORE_ERRORS
> - Changed variable names to more understandable ones
> - Added an analogue of MAX_BUFFERED_BYTES for safe buffer
>
>
> Regards,
> Damir Belyalov
> Postgres Professional
>

Regards,
Daniil Anisimov
Postgres Professional

--000000000000e2e18105ef65ed8d
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hi!</div><div><br></div><div>I have looked at your pa=
tch and have a few questions.<br><br>110: static bool SafeCopying(CopyFromS=
tate cstate, ExprContext *econtext,<br>111: 							 TupleTableSlot *myslot)=
;<br>---<br>636: bool<br>637: SafeCopying(CopyFromState cstate, ExprContext=
 *econtext, TupleTableSlot *myslot)<br><br>Why is there no static keyword i=
n the definition of the SafeCopying() function, but it is in the function d=
eclaration.<br><br>675: MemoryContext cxt =3D MemoryContextSwitchTo(econtex=
t-&gt;ecxt_per_tuple_memory);<br>676: <br>677: valid_row =3D NextCopyFrom(c=
state, econtext, myslot-&gt;tts_values, myslot-&gt;tts_isnull);<br>678: tup=
le_is_valid =3D valid_row;<br>679: <br>680: if (valid_row)<br>681: 	sfcstat=
e-&gt;safeBufferBytes +=3D cstate-&gt;line_buf.len;<br>682: <br>683: Curren=
tMemoryContext =3D cxt;<br><br>Why are you using a direct assignment to Cur=
rentMemoryContext instead of using the MemoryContextSwitchTo function in th=
e SafeCopying() routine?<br><br>1160: /* Standard copying with option &quot=
;safe copying&quot; enabled by IGNORE_ERRORS. */<br>1161: if (!SafeCopying(=
cstate, econtext, myslot))<br>1162: 	break;<br><br>I checked with GDB that =
the CurrentMemoryContext changes when SafeCopying returns. And the target c=
ontext may be different each time you do a COPY in psql.<br><br>1879: cstat=
e-&gt;sfcstate-&gt;safe_cxt =3D AllocSetContextCreate(oldcontext,<br>1880: =
							 &quot;Safe_context&quot;,<br>1881: 							 ALLOCSET_DEFAULT_SIZES);=
<br><br>When you initialize the cstate-&gt;sfcstate structure, you create a=
 cstate-&gt;sfcstate-&gt;safe_cxt memory context that inherits from oldcont=
ext.<br>Was it intended to use cstate-&gt;copycontext as the parent context=
 here?<br></div><br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gm=
ail_attr">On Wed, Nov 2, 2022 at 11:46 AM Damir Belyalov &lt;<a href=3D"mai=
lto:dam.bel07@gmail.com">dam.bel07@gmail.com</a>&gt; wrote:<br></div><block=
quote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1=
px solid rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div>Updated t=
he patch:</div><div>- Optimized and simplified logic of IGNORE_ERRORS</div>=
<div>- Changed variable names to more understandable ones</div><div>- Added=
 an analogue of MAX_BUFFERED_BYTES for safe buffer</div><div><br></div><div=
><br></div><div><div>Regards,</div><div>Damir Belyalov</div><div>Postgres P=
rofessional</div></div></div></blockquote><div><br></div><div>Regards,</div=
><div>Daniil Anisimov</div><div>Postgres Professional</div></div></div>

--000000000000e2e18105ef65ed8d--