Thread: pgsql: move hex_decode() to /common so it can be called from frontend
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(-)
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
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
Re: pgsql: move hex_decode() to /common so it can be called from frontend
From
Michael Paquier
Date:
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
Attachment
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
Attachment
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