Re: GTIN14 support for contrib/isn

Am 15.03.19 um 17:27 schrieb Tom Lane:
> Michael Kefeder <mike@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.

br
  mike