út 22. 9. 2020 v 2:33 odesílatel Tom Lane <t...@sss.pgh.pa.us> napsal:
> Pavel Stehule <pavel.steh...@gmail.com> writes: > > I see few possibilities how to finish and close this issue: > > 1. use anycompatible type and add note to documentation so expression of > > optional argument can change a result type, and so this is Postgres > > specific - other databases and ANSI SQL disallow this. > > It is the most simple solution, and it is good enough for this case, that > > is not extra important. > > 2. choose the correct type somewhere inside the parser - for these two > > functions. > > 3. introduce and implement some generic solution for this case - it can > be > > implemented on C level via some function helper or on syntax level > > CREATE OR REPLACE FUNCTION lag(a anyelement, offset int, default > defval > > a%type) ... > > "defval argname%type" means for caller's expression "CAST(defval AS > > typeof(argname))" > > I continue to feel that the spec's definition of this is not so > obviously right that we should jump through hoops to duplicate it. > In fact, I don't even agree that we need a disclaimer in the docs > saying that it's not quite the same. Only the most nitpicky > language lawyers would ever even notice. > > In hopes of moving this along a bit, I experimented with converting > the other functions I listed to use anycompatible. I soon found that > touching any functions/operators that are listed in operator classes > would open a can of worms far larger than I'd previously supposed. > To maintain consistency, we'd have to propagate the datatype changes > to *every* function/operator associated with those opfamilies --- many > of which only have one any-foo input and thus aren't on my original > list. (An example here is that extending btree array_ops will require > changing the max(anyarray) and min(anyarray) aggregates too.) We'd > then end up with a situation that would be rather confusing from a > user's standpoint, in that it'd be quite un-obvious why some array > functions use anyarray while other ones use anycompatiblearray. > > So I backed off to just changing the functions/operators that have > no opclass connections, such as array_cat. Even that has some > downsides --- for example, in the attached patch, it's necessary > to change some polymorphism.sql examples that explicitly reference > array_cat(anyarray). I wonder whether this change would break a > significant number of user-defined aggregates or operators. > > (Note that I think we'd have to resist the temptation to fix that > by letting CREATE AGGREGATE et al accept support functions that > take anyarray/anycompatiblearray (etc) interchangeably. A lot of > the security analysis that went into CVE-2020-14350 depended on > the assumption that utility commands only do exact lookups of > support functions. If we tried to be lax about this, we'd > re-introduce the possibility for hostile capture of function > references in extension scripts.) > > Another interesting issue, not seen in the attached but which > came up while I was experimenting with the more aggressive patch, > was this failure in the polymorphism test: > > select max(histogram_bounds) from pg_stats where tablename = 'pg_am'; > -ERROR: cannot compare arrays of different element types > +ERROR: function max(anyarray) does not exist > > That's because we don't accept pg_stats.histogram_bounds (of > declared type anyarray) as a valid input for anycompatiblearray. > I wonder if that isn't a bug we need to fix in the anycompatible > patch, independently of anything else. We'd not be able to deduce > an actual element type from such an input, but we already cannot > do so when we match it to an anyarray parameter. > > Anyway, attached find > > 0001 - updates Vik's original patch to HEAD and tweaks the > grammar in the docs a bit. > > 0002 - add-on patch to convert array_append, array_prepend, > array_cat, array_position, array_positions, array_remove, > array_replace, and width_bucket to use anycompatiblearray. > > I think 0001 is committable, but 0002 is just WIP since > I didn't touch the docs. I'm slightly discouraged about > whether 0002 is worth proceeding with. Any thoughts? > I think so 0002 has sense - more than doc I miss related regress tests, but it is partially covered by anycompatible tests Anyway I tested both patches and there is not problem with compilation, building doc, and make check-world passed I'll mark this patch as ready for committer Best regards Pavel > > regards, tom lane > >
