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?

> 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.

> 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

Reply via email to