SEO

#1 (**permalink**) 04-19-2008, 06:27 AM

"Brendan Jurd" <direvus@gmail.com> writes:
> As discussed on -hackers, I'm trying to get rid of some redundant code
> by creating a widely useful set of functions to convert between text
> and C string in the backend.

Applied with revisions --- the functions were modified as per recent
discussion, and I fixed a lot more potential call sites.

There are no textout/textin calls left, but I may have missed some
places that were doing it the hard way with direct palloc/memcpy
manipulations. It might be worth trolling all the VARDATA() references
to see if any more are easily replaceable.

I notice in particular that xfunc.sgml contains sample C functions to
copy and concatenate text. While these aren't directly replaceable
with the new functions, I wonder whether we ought to change the examples
to make them less certain to break if we ever change text's
representation.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

#2 (**permalink**) 04-19-2008, 06:27 AM

"Brendan Jurd" <direvus@gmail.com> writes:
> On 26/03/2008, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> There are no textout/textin calls left, but I may have missed some
>> places that were doing it the hard way with direct palloc/memcpy
>> manipulations. It might be worth trolling all the VARDATA() references
>> to see if any more are easily replaceable.

> I think there are perhaps a dozen such sites, and I hope to submit a
> cleanup patch that gets rid of these soon.
> I did come across one site I'm not sure about in utils/adt/xml.c:291

I intentionally didn't touch xml.c, nor anyplace that is not dealing
in text, even if it happens to be binary-compatible with text.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

#3 (**permalink**) 04-19-2008, 06:28 AM

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

- -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sat, Mar 29, 2008 at 5:40 AM, Brendan Jurd wrote:
> On 29/03/2008, Tom Lane wrote:
> > I intentionally didn't touch xml.c, nor anyplace that is not dealing
> > in text, even if it happens to be binary-compatible with text.
> >
>
> Hmm, okay. My original submission did include a few such changes; for
> example, in xml_in and xml_out_internal I saw that the conversion
> between xmltype and cstring was identical to the text conversion, so I
> went ahead and used the text functions. Feedback upthread suggested
> that it was okay to go ahead with casting in identical cases. [1]
>
> I saw that these changes made it into the commit, so I assumed that it
> was the right call.
>
> If we don't want to meddle with xmltype/bytea/VarChar at all, we'll
> have to revert those changes, and I'll have to seriously scale back
> the cleanup patch I was about to post.

Still not sure where we stand on the above. To cast, or not to cast?

Cheers,
BJ
- -----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIBaFy5YBsbHkuyV0RAsMmAKDHaEb7aMudKJgVxcf5RR cOtAJ+bwCgivLI
5B3xJze46NGWjEyOpq/TSaY=
=BObd
- -----END PGP SIGNATURE-----

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIBaIl5YBsbHkuyV0RArDDAJ0QThLXAhzy+NX+2YsF+q 4z/sIy1QCeJPiW
s/rVns3FFQVP5g9DTOshDfQ=
=4Tdh
-----END PGP SIGNATURE-----

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

#4 (**permalink**) 04-19-2008, 06:28 AM

"Brendan Jurd" <direvus@gmail.com> writes:
>> If we don't want to meddle with xmltype/bytea/VarChar at all, we'll
>> have to revert those changes, and I'll have to seriously scale back
>> the cleanup patch I was about to post.

> Still not sure where we stand on the above. To cast, or not to cast?

I dunno. I know there was previously some handwaving about representing
XML values in some more intelligent fashion than a plain text string,
but I have no idea if anyone is likely to do anything about it in the
foreseeable future.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

#5 (**permalink**) 04-19-2008, 06:28 AM

Tom Lane wrote:
> "Brendan Jurd" <direvus@gmail.com> writes:
>
>>> If we don't want to meddle with xmltype/bytea/VarChar at all, we'll
>>> have to revert those changes, and I'll have to seriously scale back
>>> the cleanup patch I was about to post.
>>>
>
>
>> Still not sure where we stand on the above. To cast, or not to cast?
>>
>
> I dunno. I know there was previously some handwaving about representing
> XML values in some more intelligent fashion than a plain text string,
> but I have no idea if anyone is likely to do anything about it in the
> foreseeable future.
>
>
>

It seems very unlikely to me.

cheers

andrew

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

#6 (**permalink**) 05-05-2008, 05:53 AM

"Brendan Jurd" <direvus@gmail.com> writes:
> Well ... if somebody does want to change the representation of xml
> down the road, he's going to have to touch all the sites where the
> code converts to and from cstring anyway, right?

True.

> With that in mind, please find attached my followup patch. It cleans
> up another 21 sites manually copying between cstring and varlena, for
> a net reduction of 115 lines of code.

I applied most of this, but not the parts that were dependent on the
assumption that bytea and text are the same. That is unlikely to remain
true if we ever get around to putting locale/encoding information into
text values. Furthermore it's a horrid type pun, because bytea can
contain embedded nulls whereas cstrings can't; you were making
unwarranted assumptions about whether, say, cstring_to_text_with_len
would allow embedded nulls to go by. And lastly, quite a few of those
changes were just plain broken, eg several places in selfuncs.c where
you allowed strlen() to be applied to a "bytea converted to cstring".

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

SEO

Re: [HACKERS] Text <-> C string