GTIN14 support for contrib/isn
Hello hackers,
For a project of ours we need GTIN14 data type support. The isn
extension already supports EAN13, and a '0' prefixed EAN13 is a valid
GTIN14. The leftmost "new" 14th digit is a packaging level indicator
which we need (= using EAN13 and faking a leading 0 in output doesn't
cut it).
Looking at the code I saw every format that isn-extension supports is
stored as an EAN13. Theoretically that can be changed to be GTIN14, but
that would mean quite a lot of rewrite I feared, so I chose to code only
GTIN14 I/O separetely to not interfere with any existing conversion
magic. This yields an easier to understand patch and doesn't touch
existing functionality. However it introduces redundancy to a certain
extent.
Find my patch attached. Please let me know if there are things that need
changes, I'll do my best to get GTIN support into postgresql.
thanks in advance
mike
diff --git a/expected/isn.out b/expected/isn.out
index 18fe37a..b313641 100644
--- a/expected/isn.out
+++ b/expected/isn.out
@@ -18,6 +18,8 @@ INFO: operator family "isn_ops" of access method btree is
missing cross-type op
INFO: operator family "isn_ops" of access method btree is missing cross-type
operator(s)
INFO: operator family "isn_ops" of access method btree is missing cross-type
operator(s)
INFO: operator family "isn_ops" of access method btree is missing cross-type
operator(s)
+INFO: operator family "isn_ops" of access method btree is missing cross-type
operator(s)
+INFO: operator family "isn_ops" of access method hash is missing cross-type
operator(s)
INFO: operator family "isn_ops" of access method hash is missing cross-type
operator(s)
INFO: operator family "isn_ops" of access method hash is missing cross-type
operator(s)
INFO: operator family "isn_ops" of access method hash is missing cross-type
operator(s)
@@ -29,6 +31,7 @@ INFO: operator family "isn_ops" of access method hash is
missing cross-type ope
amname | opcname
+
btree | ean13_ops
+ btree | gtin14_ops
btree | isbn13_ops
btree | isbn_ops
btree | ismn13_ops
@@ -37,6 +40,7 @@ INFO: operator family "isn_ops" of access method hash is
missing cross-type ope
btree | issn_ops
btree | upc_ops
hash | ean13_ops
+ hash | gtin14_ops
hash | isbn13_ops
hash | isbn_ops
hash | ismn13_ops
@@ -44,11 +48,20 @@ INFO: operator family "isn_ops" of access method hash is
missing cross-type ope
hash | issn13_ops
hash | issn_ops
hash | upc_ops
-(16 rows)
+(18 rows)
--
-- test valid conversions
--
+SELECT '1234567890128'::GTIN14, -- EAN is a GTIN14
+ '123456789012?'::GTIN14, -- compute check digit for me
+ '11234567890125'::GTIN14,
+ '01234567890128'::GTIN14;
+ gtin14 | gtin14 | gtin14 | gtin14
+-+-+-+-
+ 0123456789012-8 | 0123456789012-8 | 1123456789012-5 | 0123456789012-8
+(1 row)
+
SELECT '9780123456786'::EAN13, -- old book
'9790123456785'::EAN13, -- music
'9791234567896'::EAN13, -- new book
@@ -249,6 +262,67 @@ SELECT 9780123456786::ISBN;
ERROR: cannot cast type bigint to isbn
LINE 1: SELECT 9780123456786::ISBN;
^
+SELECT '91234567890125'::GTIN14; -- invalid indicator
+ERROR: Indicator digit out of range for GTIN14 number: "91234567890125"
+LINE 1: SELECT '91234567890125'::GTIN14;
+ ^
+SELECT '123456789012'::GTIN14; -- too short
+ERROR: invalid input syntax for GTIN14 number: "123456789012"
+LINE 1: SELECT '123456789012'::GTIN14;
+ ^
+SELECT '1234567890127'::GTIN14; -- wrong checkdigit
+ERROR: invalid check digit for GTIN14 number: "1234567890127", should be 8
+LINE 1: SELECT '1234567890127'::GTIN14;
+ ^
+--
+-- test validity helpers
+--
+SELECT make_valid('1234567890120!'::GTIN14); -- EAN-13
+ make_valid
+-
+ 0123456789012-8
+(1 row)
+
+SELECT make_valid('11234567890120!'::GTIN14); -- GTIN-14
+ make_valid
+-
+ 1123456789012-5
+(1 row)
+
+SELECT is_valid(make_valid('1234567890120!'::GTIN14)); -- EAN-13
+ is_valid
+--
+ t
+(1 row)
+
+SELECT is_valid(make_valid('11234567890120!'::GTIN14)); -- GTIN-14
+ is_valid
+--
+ t
+(1 row)
+
+CREATE TABLE gtin_valid (gtin GTIN14 NOT NULL);
+INSERT INTO gtin_valid VALUES
+-- all invalid because of ! marking
+ ('1234567890120!'), -- invalid EAN-13
+ ('1234567890128!'), -- valid EAN-13
+ ('11234567890120!'), -- invalid GTIN-14
+ ('11234567890125!'), -- valid GTIN-14
+-- valid
+ ('1234567890128'::GTIN14), -- valid EAN-13
+ ('11234567890125'::GTIN14); -- valid GTIN-14
+SELECT gtin, is_valid(gtin) FROM gtin_valid;
+ gtin | is_valid
+--+--
+ 0123456789012-8! | f
+ 0123456789012-8! | f
+ 1123456789012-5! | f
+ 1123456789012-5! | f
+ 0123456789012-8 | t
+ 1123456789012-5 | t
+(6 rows)
+
+DROP T
Re: GTIN14 support for contrib/isn
Am 15.03.19 um 17:27 schrieb Tom Lane: Michael Kefeder writes: For a project of ours we need GTIN14 data type support. Hm, what is that and where would a reviewer find the specification for it? specs are from GS1 here https://www.gs1.org/standards/id-keys/gtin side-note EAN13 is actually called GTIN-13 now. Wikipedia has a quick overview https://en.wikipedia.org/wiki/Global_Trade_Item_Number Looking at the code I saw every format that isn-extension supports is stored as an EAN13. Theoretically that can be changed to be GTIN14, but that would mean quite a lot of rewrite I feared, so I chose to code only GTIN14 I/O separetely to not interfere with any existing conversion magic. This yields an easier to understand patch and doesn't touch existing functionality. However it introduces redundancy to a certain extent. Yeah, you certainly don't get to change the on-disk format of the existing types, unfortunately. Not sure what the least messy way of dealing with that is. I guess we do want this to be part of contrib/isn rather than an independent module, if there are sane datatype conversions with the existing isn types. the on-disk format does not change (it would support even longer codes it's just an integer where one bit is used for valid/invalid flag, did not touch that at all). Putting GTIN14 in isn makes sense I find and is back/forward compatible. Find my patch attached. Please let me know if there are things that need changes, I'll do my best to get GTIN support into postgresql. Well, two comments immediately: * where's the documentation changes? * simply editing the .sql file in-place is not acceptable; that breaks the versioning conventions for extensions, and leaves users with no easy upgrade path. What you need to do is create a version upgrade script that adds the new objects. For examples look for other recent patches that have added features to contrib modules, eg https://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=eb6f29141bed9dc95cb473614c30f470ef980705 Also, I'm afraid you've pretty much missed the deadline to get this into PG v12; we've already got more timely-submitted patches than we're likely to be able to finish reviewing. Please add it to the first v13 commit fest, https://commitfest.postgresql.org/23/ so that we don't forget about it when the time does come to look at it. regards, tom lane thanks for the feedback! will do mentioned documentation changes and create a separate upgrade sql file. Making it into v13 is fine by me. br mike
Re: GTIN14 support for contrib/isn
--- Original Message --- On Thursday, June 8th, 2023 at 5:23 PM, Josef Šimánek wrote: > čt 8. 6. 2023 v 17:20 odesílatel Michael Kefeder m...@multiwave.ch napsal: > > > Am 15.03.19 um 17:27 schrieb Tom Lane: > > > > > Michael Kefeder m...@multiwave.ch writes: > > > > > > > For a project of ours we need GTIN14 data type support. > > > > > > Hm, what is that and where would a reviewer find the specification for it? > > > > specs are from GS1 here https://www.gs1.org/standards/id-keys/gtin > > side-note EAN13 is actually called GTIN-13 now. Wikipedia has a quick > > overview https://en.wikipedia.org/wiki/Global_Trade_Item_Number > > > > > > Looking at the code I saw every format that isn-extension supports is > > > > stored as an EAN13. Theoretically that can be changed to be GTIN14, but > > > > that would mean quite a lot of rewrite I feared, so I chose to code only > > > > GTIN14 I/O separetely to not interfere with any existing conversion > > > > magic. This yields an easier to understand patch and doesn't touch > > > > existing functionality. However it introduces redundancy to a certain > > > > extent. > > > > > > Yeah, you certainly don't get to change the on-disk format of the existing > > > types, unfortunately. Not sure what the least messy way of dealing with > > > that is. I guess we do want this to be part of contrib/isn rather than > > > an independent module, if there are sane datatype conversions with the > > > existing isn types. > > > > the on-disk format does not change (it would support even longer codes > > it's just an integer where one bit is used for valid/invalid flag, did > > not touch that at all). Putting GTIN14 in isn makes sense I find and is > > back/forward compatible. > > > > > > Find my patch attached. Please let me know if there are things that need > > > > changes, I'll do my best to get GTIN support into postgresql. > > > > > > Well, two comments immediately: > > > > > > * where's the documentation changes? > > > > > > * simply editing the .sql file in-place is not acceptable; that breaks > > > the versioning conventions for extensions, and leaves users with no > > > easy upgrade path. What you need to do is create a version upgrade > > > script that adds the new objects. For examples look for other recent > > > patches that have added features to contrib modules, eg > > > > > > https://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=eb6f29141bed9dc95cb473614c30f470ef980705 > > > > > > Also, I'm afraid you've pretty much missed the deadline to get this > > > into PG v12; we've already got more timely-submitted patches than > > > we're likely to be able to finish reviewing. Please add it to the > > > first v13 commit fest, > > > > > > https://commitfest.postgresql.org/23/ > > > > > > so that we don't forget about it when the time does come to look at it. > > > > > > regards, tom lane > > > > thanks for the feedback! will do mentioned documentation changes and > > create a separate upgrade sql file. Making it into v13 is fine by me. > > > Hello! > > If I understand it well, this patch wasn't finished and submitted > after this discussion. If there is still interest, I can try to polish > the patch, rebase and submit. I'm interested in GTIN14 support. > Hello Josef, >From my side you can finish the patch. Sorry that I didn't follow up on it, >the company completely switched product line and then I forgot about it >because we no longer needed it. br mike
