From: Viktor Holmberg <v@viktorh.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: ISN extension - wrong volatility level for isn_weak() function
Date: Sat, 15 Mar 2025 21:17:16 +0000
Message-ID: <b08ed3b6-eba7-47f5-b354-5e4ec1fb8632@Spark> (raw)
In-Reply-To: <3481904.1742055723@sss.pgh.pa.us>
References: <ae6ed432-4491-4030-be20-3ea69d03a098@Spark>
<790bc1f9-74dc-4b50-94d2-8147315b1556@Spark>
<51C655A4-46FB-4A40-BAE5-26A76B31D3EE@yesql.se>
<3052531.1741961784@sss.pgh.pa.us>
<de89bd5c-9f51-4891-8a38-d03ed06ebd1d@Spark>
<3074065.1741965229@sss.pgh.pa.us>
<b698bb0e-766a-4255-a8c2-a1d14ef2a8f5@Spark>
<3080651.1741969122@sss.pgh.pa.us>
<742add6c-02ac-42e4-97a3-f2ebd1520d64@Spark>
<3481904.1742055723@sss.pgh.pa.us>
Oh, sorry! Don’t know what I was doing there - not used to this patch based workflow. Here comes the real patch.
This now uses the GUC - that was a lot easier than I thought.
One thing I couldn’t figure out: Should I add some sort of change log somewhere, describing what changed in version 1.3 of the ISN extension? If so, where?
/Viktor Holmberg
On 15 Mar 2025 at 16:22 +0000, Tom Lane <tgl@sss.pgh.pa.us>, wrote:
> Viktor Holmberg <v@viktorh.net> writes:
> > On 14 Mar 2025 at 16:18 +0000, Tom Lane <tgl@sss.pgh.pa.us>, wrote:
> > > No, we'd need to create an update script that uses ALTER FUNCTION.
> > > Extension scripts are basically frozen once shipped.
>
> > Ah, thanks for the clarification. I’ve attached a patch that fixes the volatility. I thought it best to at least get some feedback on that before I try to dust off my C knowledge and try to fix the GUC stuff.
>
> Uh .. looks like you attached a patch for something else altogether.
>
> If you want a sample extension-updating patch to look at, you
> could see 44ba2920644903d7dfceda810e5facdbcbab58a8, or lots of
> other examples.
>
> regards, tom lane
Attachments:0001-isn-extension-Make-weak-mode-a-GUC-setting.patch (application/octet-stream, 12.9K) download
From d3919051536729f3b17792bedfc92dd1cac1505b Mon Sep 17 00:00:00 2001
From: Viktor Holmberg <v@viktorh.net>
Date: Sat, 15 Mar 2025 21:07:01 +0000
Subject: [PATCH] isn extension: Make weak mode a GUC setting
isn_weak mode used to be settable only using the isn_weak(boolean) function.
This wasn't optimal, as this means it doesn't respect transactions.
This change makes the isn.weak a GUC parameter instead.
The isn_weak functions are retained for backwards compatibility.
The also had a bug where they were marked IMMUTABLE, when in fact
they are VOLATILE. This is also fixed.
---
contrib/isn/Makefile | 2 +-
contrib/isn/expected/isn.out | 11 +++
contrib/isn/isn--1.2--1.3.sql | 7 ++
contrib/isn/isn.c | 18 +++--
contrib/isn/isn.control | 2 +-
contrib/isn/isn.h | 1 -
contrib/isn/sql/isn.sql | 5 ++
doc/src/sgml/isn.sgml | 139 +++++++++++++++++++---------------
8 files changed, 118 insertions(+), 67 deletions(-)
create mode 100644 contrib/isn/isn--1.2--1.3.sql
diff --git a/contrib/isn/Makefile b/contrib/isn/Makefileindex 1037506c705..35d9249f3a7 100644--- a/contrib/isn/Makefile+++ b/contrib/isn/Makefile@@ -4,7 +4,7 @@ MODULES = isn
EXTENSION = isn
DATA = isn--1.1.sql isn--1.1--1.2.sql \
- isn--1.0--1.1.sql+ isn--1.0--1.1.sql isn--1.2--1.3.sql
PGFILEDESC = "isn - data types for international product numbering standards"
# the other .h files are data tables, we don't install those
diff --git a/contrib/isn/expected/isn.out b/contrib/isn/expected/isn.outindex 2f05b7eb861..b7addb508dd 100644--- a/contrib/isn/expected/isn.out+++ b/contrib/isn/expected/isn.out@@ -279,6 +279,17 @@ FROM (VALUES ('9780123456786', 'UPC'),
9771234567003 | ISSN | t | | | |
(3 rows)
+SELECT '2222222222221'::ean13;+ERROR: invalid check digit for EAN13 number: "2222222222221", should be 2+LINE 1: SELECT '2222222222221'::ean13;+ ^+SET isn.weak TO TRUE;+SELECT '2222222222221'::ean13;+ ean13 +------------------+ 222-222222222-2!+(1 row)+
--
-- cleanup
--
diff --git a/contrib/isn/isn--1.2--1.3.sql b/contrib/isn/isn--1.2--1.3.sqlnew file mode 100644index 00000000000..26d1662c2b8--- /dev/null+++ b/contrib/isn/isn--1.2--1.3.sql@@ -0,0 +1,7 @@+ALTER FUNCTION isn_weak() VOLATILE;+ALTER FUNCTION isn_weak(boolean) VOLATILE;++COMMENT ON FUNCTION isn_weak(boolean) IS+'Sets the weak input mode, and returns the new setting. This function is retained for backward compatibility. The recommended way to set weak mode is via the isn.weak GUC parameter.';+COMMENT ON FUNCTION isn_weak() IS+'Returns the current status of the weak mode. This function is retained for backward compatibility. Users should check the value of isn.weak instead.';diff --git a/contrib/isn/isn.c b/contrib/isn/isn.cindex db765ee490d..82c239f0ec1 100644--- a/contrib/isn/isn.c+++ b/contrib/isn/isn.c@@ -21,6 +21,7 @@
#include "UPC.h"
#include "fmgr.h"
#include "isn.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
@@ -929,6 +930,18 @@ _PG_init(void)
if (!check_table(UPC_range, UPC_index))
elog(ERROR, "UPC failed check");
}
++ /* Define a GUC variable for weak mode. */+ DefineCustomBoolVariable("isn.weak",+ "Accept invalid isns",+ "When enabled, invalid numbers are accepted but flagged with an exclamation mark (!), indicating an incorrect check digit.",+ &g_weak,+ false,+ PGC_USERSET,+ 0,+ NULL,+ NULL,+ NULL);
}
/* isn_out
@@ -1109,17 +1122,12 @@ make_valid(PG_FUNCTION_ARGS)
/* this function temporarily sets weak input flag
* (to lose the strictness of check digit acceptance)
- * It's a helper function, not intended to be used!!
*/
PG_FUNCTION_INFO_V1(accept_weak_input);
Datum
accept_weak_input(PG_FUNCTION_ARGS)
{
-#ifdef ISN_WEAK_MODE
g_weak = PG_GETARG_BOOL(0);
-#else- /* function has no effect */-#endif /* ISN_WEAK_MODE */
PG_RETURN_BOOL(g_weak);
}
diff --git a/contrib/isn/isn.control b/contrib/isn/isn.controlindex 1cb5e2b2340..e7daea52b84 100644--- a/contrib/isn/isn.control+++ b/contrib/isn/isn.control@@ -1,6 +1,6 @@
# isn extension
comment = 'data types for international product numbering standards'
-default_version = '1.2'+default_version = '1.3'
module_pathname = '$libdir/isn'
relocatable = true
trusted = true
diff --git a/contrib/isn/isn.h b/contrib/isn/isn.hindex 038eb362c39..399896ad417 100644--- a/contrib/isn/isn.h+++ b/contrib/isn/isn.h@@ -18,7 +18,6 @@
#include "fmgr.h"
#undef ISN_DEBUG
-#define ISN_WEAK_MODE
/*
* uint64 is the internal storage format for ISNs.
diff --git a/contrib/isn/sql/isn.sql b/contrib/isn/sql/isn.sqlindex 2c2ea077d1e..3154dc7a8e7 100644--- a/contrib/isn/sql/isn.sql+++ b/contrib/isn/sql/isn.sql@@ -120,6 +120,11 @@ FROM (VALUES ('9780123456786', 'UPC'),
AS a(str,typ),
LATERAL pg_input_error_info(a.str, a.typ) as errinfo;
+SELECT '2222222222221'::ean13;+SET isn.weak TO TRUE;+SELECT '2222222222221'::ean13;++
--
-- cleanup
--
diff --git a/doc/src/sgml/isn.sgml b/doc/src/sgml/isn.sgmlindex 45a867d98c2..75d5afd26b2 100644--- a/doc/src/sgml/isn.sgml+++ b/doc/src/sgml/isn.sgml@@ -224,15 +224,72 @@
</para>
</sect2>
+ <sect2 id="isn-guc-params">+ <title>GUC Parameters</title>+ <variablelist>+ <varlistentry id="guc-isn-weak">+ <term>+ <varname>isn.weak</varname> (<type>boolean</type>)+ <indexterm>+ <primary><varname>isn.weak</varname> configuration parameter</primary>+ </indexterm>+ </term>+ <listitem>+ <para>+ Sets the weak input mode. Default: false (strong mode)+ </para>+ </listitem>+ </varlistentry>+ </variablelist>+ <para>+ <firstterm>Weak</firstterm> mode is used to be able to insert invalid data+ into a table. Invalid means the check digit is wrong, not that there are+ missing numbers.+ </para>++ <para>+ Why would you want to use the weak mode? Well, it could be that+ you have a huge collection of ISBN numbers, and that there are so many of+ them that for weird reasons some have the wrong check digit (perhaps the+ numbers were scanned from a printed list and the OCR got the numbers wrong,+ perhaps the numbers were manually captured... who knows). Anyway, the point+ is you might want to clean the mess up, but you still want to be able to+ have all the numbers in your database and maybe use an external tool to+ locate the invalid numbers in the database so you can verify the+ information and validate it more easily; so for example you'd want to+ select all the invalid numbers in the table.+ </para>++ <para>+ When you insert invalid numbers in a table using the weak mode, the number+ will be inserted with the corrected check digit, but it will be displayed+ with an exclamation mark (<literal>!</literal>) at the end, for example+ <literal>0-11-000322-5!</literal>. This invalid marker can be checked with+ the <function>is_valid</function> function and cleared with the+ <function>make_valid</function> function.+ </para>++ <para>+ You can also force the insertion of invalid numbers even when not in the+ weak mode, by appending the <literal>!</literal> character at the end of the+ number.+ </para>++ <para>+ Another special feature is that during input, you can write+ <literal>?</literal> in place of the check digit, and the correct check digit+ will be inserted automatically.+ </para>+</sect2>+
<sect2 id="isn-funcs-ops">
<title>Functions and Operators</title>
<para>
The <filename>isn</filename> module provides the standard comparison operators,
- plus B-tree and hash indexing support for all these data types. In- addition there are several specialized functions; shown in <xref linkend="isn-functions"/>.- In this table,- <type>isn</type> means any one of the module's data types.+ plus B-tree and hash indexing support for all these data types. In+ addition, there are several specialized functions, shown in <xref linkend="isn-functions"/>.+ In this table, <type>isn</type> means any one of the module's data types.
</para>
<table id="isn-functions">
@@ -252,89 +309,53 @@
<tbody>
<row>
<entry role="func_table_entry"><para role="func_signature">
- <indexterm><primary>isn_weak</primary></indexterm>- <function>isn_weak</function> ( <type>boolean</type> )- <returnvalue>boolean</returnvalue>+ <indexterm><primary>make_valid</primary></indexterm>+ <function>make_valid</function> ( <type>isn</type> )+ <returnvalue>isn</returnvalue>
</para>
<para>
- Sets the weak input mode, and returns new setting.+ Validates an invalid number (clears the invalid flag).
</para></entry>
</row>
<row>
<entry role="func_table_entry"><para role="func_signature">
- <function>isn_weak</function> ()+ <indexterm><primary>is_valid</primary></indexterm>+ <function>is_valid</function> ( <type>isn</type> )
<returnvalue>boolean</returnvalue>
</para>
<para>
- Returns the current status of the weak mode.+ Checks for the presence of the invalid flag.
</para></entry>
</row>
<row>
<entry role="func_table_entry"><para role="func_signature">
- <indexterm><primary>make_valid</primary></indexterm>- <function>make_valid</function> ( <type>isn</type> )- <returnvalue>isn</returnvalue>+ <indexterm><primary>isn_weak</primary></indexterm>+ <function>isn_weak</function> ( <type>boolean</type> )+ <returnvalue>boolean</returnvalue>
</para>
<para>
- Validates an invalid number (clears the invalid flag).+ Sets the weak input mode, and returns the new setting.+ This function is retained for backward compatibility. + The recommended way to set weak mode is via the <varname>isn.weak</varname> GUC parameter.
</para></entry>
</row>
<row>
<entry role="func_table_entry"><para role="func_signature">
- <indexterm><primary>is_valid</primary></indexterm>- <function>is_valid</function> ( <type>isn</type> )+ <function>isn_weak</function> ()
<returnvalue>boolean</returnvalue>
</para>
<para>
- Checks for the presence of the invalid flag.+ Returns the current status of the weak mode.+ This function is retained for backward compatibility.+ Users should check the value of <varname>isn.weak</varname> instead.
</para></entry>
</row>
</tbody>
</tgroup>
</table>
-- <para>- <firstterm>Weak</firstterm> mode is used to be able to insert invalid data- into a table. Invalid means the check digit is wrong, not that there are- missing numbers.- </para>-- <para>- Why would you want to use the weak mode? Well, it could be that- you have a huge collection of ISBN numbers, and that there are so many of- them that for weird reasons some have the wrong check digit (perhaps the- numbers were scanned from a printed list and the OCR got the numbers wrong,- perhaps the numbers were manually captured... who knows). Anyway, the point- is you might want to clean the mess up, but you still want to be able to- have all the numbers in your database and maybe use an external tool to- locate the invalid numbers in the database so you can verify the- information and validate it more easily; so for example you'd want to- select all the invalid numbers in the table.- </para>-- <para>- When you insert invalid numbers in a table using the weak mode, the number- will be inserted with the corrected check digit, but it will be displayed- with an exclamation mark (<literal>!</literal>) at the end, for example- <literal>0-11-000322-5!</literal>. This invalid marker can be checked with- the <function>is_valid</function> function and cleared with the- <function>make_valid</function> function.- </para>-- <para>- You can also force the insertion of invalid numbers even when not in the- weak mode, by appending the <literal>!</literal> character at the end of the- number.- </para>-- <para>- Another special feature is that during input, you can write- <literal>?</literal> in place of the check digit, and the correct check digit- will be inserted automatically.- </para>
</sect2>
<sect2 id="isn-examples">
@@ -366,11 +387,11 @@ SELECT issn('3251231?');
SELECT ismn('979047213542?');
--Using the weak mode:
-SELECT isn_weak(true);+SET isn.weak TO true;
INSERT INTO test VALUES('978-0-11-000533-4');
INSERT INTO test VALUES('9780141219307');
INSERT INTO test VALUES('2-205-00876-X');
-SELECT isn_weak(false);+SET isn.weak TO false;
SELECT id FROM test WHERE NOT is_valid(id);
UPDATE test SET id = make_valid(id) WHERE id = '2-205-00876-X!';
--
2.48.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: v@viktorh.net, tgl@sss.pgh.pa.us, daniel@yesql.se, pgsql-bugs@lists.postgresql.org
Subject: Re: ISN extension - wrong volatility level for isn_weak() function
In-Reply-To: <b08ed3b6-eba7-47f5-b354-5e4ec1fb8632@Spark>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox