Re: pgsql-server/src/backend commands/typecmds.c e ... - Mailing list pgsql-committers

Tom Lane writes:

> >> ereport(ERROR,
> >> !                         (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> >> errmsg("set-valued function called in context that cannot accept a set")));
>
> >> ereport(ERROR,
> >> !                         (errcode(ERRCODE_SYNTAX_ERROR),
> >> errmsg("set-valued function called in context that cannot accept a set")));
>
> > This seems kind of obvious.  How would a "supported feature" that allows
> > set-valued functions to be called in contexts that cannot accept a set
> > look like.
>
> The aspect of it that's unsupported is that the particular context
> (which in this case really means a particular caller of ExecEvalExpr)
> isn't prepared to deal with a set-valued result.  It seems entirely
> likely to me that we might someday improve some of those callers to
> allow sets; that's why I marked this as an unsupported feature.
> Syntax error does not seem appropriate.  The same goes for the other
> occurrences of the same error string.

Changed back to unsupported feature.

> >> ereport(ERROR,
> >> !                 (errcode(ERRCODE_SYNTAX_ERROR),
> >> errmsg("cannot specify both user and group")));
> >> }
>
> > This was never a feature, it was a definitional impossibility.
>
> Mph, maybe that should go back to being an elog (ie, internal error).
> Is it possible for control to get here?

Yes, it happens when you call the function as makeaclitem(0, 0, ...).
I've changed it to DATA_EXCEPTION.  (A more specific code doesn't seem
worthwhile, because this function is mainly for information schema
support.)

> >> ereport(ERROR,
> >> !                 (errcode(ERRCODE_SYNTAX_ERROR),
> >> !                  errmsg("argument must be empty or one-dimensional array")));
>
> > Push works on a stack, and a stack is always one-dimensional.  It's never
> > going to be supported under the label "push".  Maybe a separate error
> > along the lines of "cardinality error" would be appropriate.
>
> This is certainly not a syntax error; it belongs in the data-exception
> SQLSTATE category.  (AFAICT, "cardinality violation" is supposed to be
> reserved for wrong numbers of *rows* in table results, which this
> isn't.)  ARRAY_SUBSCRIPT_ERROR would be fine with me; I think we use
> that in other contexts where the size of an array isn't right.

That would be inappropriate, because it's the dimension, not the size.
In fact, you can view our multidimensional arrays as arrays of arrays in
SQL, so it really boils down to a run-time type mismatch.  I've made it
DATA_EXCEPTION for now.

> >> !                     (errcode(ERRCODE_INTERNAL_ERROR),
> >> !                      errmsg("invalid constraint type \"%c\"",
> >> conForm->contype)));
>
> > This error speaks of bogus values in a system table.
>
> I'd suggest reverting such things to elog() style, so that translators
> don't have to deal with those messages.

Done.

> >> ereport(ERROR,
> >> !                 (errcode(ERRCODE_SYNTAX_ERROR),
> >> errmsg("cannot XOR bit strings of different sizes")));
> >>
> >> len = VARSIZE(arg1);
>
> > We once decided (for better or worse) that bit string operations would
> > only be defined for certain operators.
>
> These again should be data errors not syntax errors.  Perhaps
> STRING_DATA_LENGTH_MISMATCH?  The SQL spec seems to use "string" to
> cover both character and bit strings in some places, so it seems
> reasonable to use that SQLSTATE for this.

Done.  In fact, the only standard use I see for "string data, length
mismatch" is for bit strings.

--
Peter Eisentraut   peter_e@gmx.net