pgsql: move hex_decode() to /common so it can be called from frontend

Started by Bruce Momjianover 5 years ago7 messagescomitters
Jump to latest
#1Bruce Momjian
bruce@momjian.us

move hex_decode() to /common so it can be called from frontend

This allows removal of a copy of hex_decode() from ecpg, and will be
used by the soon-to-be added pg_alterckey command.

Backpatch-through: master

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/c3826f831e6e63e13a749fd3ab9fd7106707b549

Modified Files
--------------
src/backend/utils/adt/encode.c | 64 +---------------------
src/backend/utils/adt/varlena.c | 1 +
src/common/Makefile | 1 +
src/common/hex_decode.c | 106 +++++++++++++++++++++++++++++++++++++
src/include/common/hex_decode.h | 16 ++++++
src/include/utils/builtins.h | 1 -
src/interfaces/ecpg/ecpglib/data.c | 52 +-----------------
src/tools/msvc/Mkvcbuild.pm | 2 +-
8 files changed, 127 insertions(+), 116 deletions(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: pgsql: move hex_decode() to /common so it can be called from frontend

Bruce Momjian <bruce@momjian.us> writes:

move hex_decode() to /common so it can be called from frontend

The buildfarm seems pretty unimpressed with this.

regards, tom lane

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: pgsql: move hex_decode() to /common so it can be called from frontend

On Thu, Dec 24, 2020 at 06:14:49PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

move hex_decode() to /common so it can be called from frontend

The buildfarm seems pretty unimpressed with this.

Yes, we are working on reverting the ecpg part.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

#4Michael Paquier
michael@paquier.xyz
In reply to: Bruce Momjian (#3)
Re: pgsql: move hex_decode() to /common so it can be called from frontend

On Thu, Dec 24, 2020 at 06:16:20PM -0500, Bruce Momjian wrote:

On Thu, Dec 24, 2020 at 06:14:49PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

move hex_decode() to /common so it can be called from frontend

The buildfarm seems pretty unimpressed with this.

Yes, we are working on reverting the ecpg part.

Looks like the defense put in place by 6b1c5ca has allowed to catch up
a bug here. When base64 has been copied from encode.c to src/common/
for SCRAM (newlines should not be handled by SCRAM, hence the copy),
we have done the same. The copied code just returns -1 for error
paths. For this case, I think that you should also prefix those
functions with "pg_", and also include the encode part for
completeness.
--
Michael

#5Bruce Momjian
bruce@momjian.us
In reply to: Michael Paquier (#4)
Re: pgsql: move hex_decode() to /common so it can be called from frontend

On Fri, Dec 25, 2020 at 09:04:41AM +0900, Michael Paquier wrote:

Looks like the defense put in place by 6b1c5ca has allowed to catch up
a bug here. When base64 has been copied from encode.c to src/common/
for SCRAM (newlines should not be handled by SCRAM, hence the copy),
we have done the same. The copied code just returns -1 for error
paths. For this case, I think that you should also prefix those
functions with "pg_", and also include the encode part for
completeness.

I now understand the wisdom of your suggestion. Attached is a patch
that removes hex_decode from ecpg properly, and returns -1 from the
/common version.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

Attachments:

hex.difftext/x-diff; charset=us-asciiDownload+14-64
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#5)
Re: pgsql: move hex_decode() to /common so it can be called from frontend

Bruce Momjian <bruce@momjian.us> writes:

I now understand the wisdom of your suggestion. Attached is a patch
that removes hex_decode from ecpg properly, and returns -1 from the
/common version.

I'm fairly unimpressed with this. I don't like having fundamentally
different (and 100% undocumented) behaviors between the frontend and
backend versions of the "same" function.

I think you should leave ecpglib alone. Unifying that little bit
of code isn't worth having to contort the API of the common version.

regards, tom lane

#7Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#6)
Re: pgsql: move hex_decode() to /common so it can be called from frontend

On Thu, Dec 24, 2020 at 09:20:27PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

I now understand the wisdom of your suggestion. Attached is a patch
that removes hex_decode from ecpg properly, and returns -1 from the
/common version.

I'm fairly unimpressed with this. I don't like having fundamentally
different (and 100% undocumented) behaviors between the frontend and
backend versions of the "same" function.

I think you should leave ecpglib alone. Unifying that little bit
of code isn't worth having to contort the API of the common version.

OK, sure, I can see that point too.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee