Thread: src/backend/parser/parse_expr.c:exprTypmod() question

src/backend/parser/parse_expr.c:exprTypmod() question

From
Teodor Sigaev
Date:
I'm working on user-defined typmod and try to move all typmod calculations into 
type-specific functions. But there is a strange place:

/* *  exprTypmod - *    returns the type-specific attrmod of the expression, if it can be *    determined.  In most
cases,it can't and we return -1. */
 
int32
exprTypmod(Node *expr)
{
<skip>        case T_Const:            {                /* Be smart about string constants... */                Const
  *con = (Const *) expr;
 
                switch (con->consttype)                {                    case BPCHAROID:                        if
(!con->constisnull)                       {                            int32       len = 
 
VARSIZE(DatumGetPointer(con->constvalue)) - VARHDRSZ;
                            /* if multi-byte, take len and find # characters */                            if
(pg_database_encoding_max_length()> 1)                                len = 
 
pg_mbstrlen_with_len(VARDATA(DatumGetPointer(con->constvalue)), len);                            return len + VARHDRSZ;
                      }                        break;                    default:                        break;
      }            }            break;
 


So, I can't understand why it's needed at all. First, it's returns length as 
typmod, second, it looks like optimization, but I don't believe in significant 
benefits... It's a constant coming from query. Am I missing something?



-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
  WWW: http://www.sigaev.ru/
 


Re: src/backend/parser/parse_expr.c:exprTypmod() question

From
Gregory Stark
Date:
Teodor Sigaev <teodor@sigaev.ru> writes:

> I'm working on user-defined typmod and try to move all typmod calculations into
> type-specific functions. But there is a strange place:
>
> /*
>  *  exprTypmod -
>  *    returns the type-specific attrmod of the expression, if it can be
>  *    determined.  In most cases, it can't and we return -1.
>  */
...
> So, I can't understand why it's needed at all. First, it's returns length as
> typmod, second, it looks like optimization, but I don't believe in significant
> benefits... It's a constant coming from query. Am I missing something?

I think that comes into play in cases like the following:

postgres=# create table qux as (select 'foo'::bpchar, 'foo'::varchar, 0::numeric);
SELECT
postgres=# \d qux          Table "public.qux"Column  |       Type        | Modifiers 
---------+-------------------+-----------bpchar  | character(3)      | varchar | character varying | numeric | numeric
        | 
 


Note that unlike most of the built-in types bpchar doesn't actually make much
sense without a typmod. NUMERIC, VARCHAR, etc can all exist without a typmod
and behave sensibly but bpchar without a typmod would just be a varchar. The
default for CHARACTER without a typmod is CHAR(1) which is what happens if you
do ::CHAR but I guess we don't want to do that for ::bpchar.

On the other hand I can manually create a table with a column of type bpchar
and it does behave like a varchar with strange comparison semantics so I guess
you could argue bpchar without a typmod isn't completely meaningless.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com


Re: src/backend/parser/parse_expr.c:exprTypmod() question

From
Tom Lane
Date:
Gregory Stark <stark@enterprisedb.com> writes:
> Teodor Sigaev <teodor@sigaev.ru> writes:
>> I'm working on user-defined typmod and try to move all typmod calculations into
>> type-specific functions. But there is a strange place:

> Note that unlike most of the built-in types bpchar doesn't actually make much
> sense without a typmod.

You may be reading too much into it.  Looking at the patch that
introduced exprTypmod(), I think I may have just been interested
in avoiding an unnecessary length-coercion function call when assigning
a constant that was already of the correct length to a CHAR(N) column.
I concur with Teodor that embedding this type-specific knowledge into
exprTypmod probably isn't all that great an idea.
        regards, tom lane