DEFAULT fixed - Mailing list pgsql-hackers

I have fixed the problem with DEFAULT ''.

    test=> create table t1 (str1 char(2) default '',str2 text default
    '',str3 text default '' );
    CREATE
    test=> insert into t1 values ('aa', 'string2', 'string3');
    INSERT 18830 1
    test=> insert into t1 (str3) values ('string3');
    INSERT 18831 1
    test=> select * from t1;
    str1|str2   |str3
    ----+-------+-------
    aa  |string2|string3
        |       |string3
    (2 rows)

The fix is to pass atttypmod into parse_coerce(), and when a bpchar type
is the output type, we pass the atttypmod value into bpcharin, so the
type is properly padded.

The bad news is that many other calls to parse_coerce do not have access
to the column's atttypmod value, so they don't properly pad the
coersion.

Does anyone have an opinion on this?  Why does only DEFAULT have this
problem?  Does anyone know how inserts of '' into char() fields get
padded with the proper atttypmod value?  Do I need to pass atttypmod to
all the functions that call parse_coerce, so I can pass a value for all
cases?

--
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
? src/Makefile.custom
? src/config.log
? src/log
? src/config.cache
? src/config.status
? src/GNUmakefile
? src/Makefile.global
? src/backend/fmgr.h
? src/backend/parse.h
? src/backend/postgres
? src/backend/global1.bki.source
? src/backend/local1_template1.bki.source
? src/backend/global1.description
? src/backend/local1_template1.description
? src/backend/bootstrap/bootparse.c
? src/backend/bootstrap/bootstrap_tokens.h
? src/backend/bootstrap/bootscanner.c
? src/backend/catalog/genbki.sh
? src/backend/catalog/global1.bki.source
? src/backend/catalog/global1.description
? src/backend/catalog/local1_template1.bki.source
? src/backend/catalog/local1_template1.description
? src/backend/port/Makefile
? src/backend/utils/Gen_fmgrtab.sh
? src/backend/utils/fmgr.h
? src/backend/utils/fmgrtab.c
? src/bin/cleardbdir/cleardbdir
? src/bin/createdb/createdb
? src/bin/createlang/createlang
? src/bin/createuser/createuser
? src/bin/destroydb/destroydb
? src/bin/destroylang/destroylang
? src/bin/destroyuser/destroyuser
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_dump/Makefile
? src/bin/pg_dump/pg_dump
? src/bin/pg_id/pg_id
? src/bin/pg_passwd/pg_passwd
? src/bin/pg_version/Makefile
? src/bin/pg_version/pg_version
? src/bin/pgtclsh/mkMakefile.tcldefs.sh
? src/bin/pgtclsh/mkMakefile.tkdefs.sh
? src/bin/pgtclsh/Makefile.tkdefs
? src/bin/pgtclsh/Makefile.tcldefs
? src/bin/pgtclsh/pgtclsh
? src/bin/pgtclsh/pgtksh
? src/bin/psql/Makefile
? src/bin/psql/psql
? src/include/version.h
? src/include/config.h
? src/interfaces/ecpg/lib/Makefile
? src/interfaces/ecpg/lib/libecpg.so.3.0.0
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpgtcl/Makefile
? src/interfaces/libpgtcl/libpgtcl.so.2.0
? src/interfaces/libpq/Makefile
? src/interfaces/libpq/libpq.so.2.0
? src/interfaces/libpq++/Makefile
? src/interfaces/libpq++/libpq++.so.2.0
? src/interfaces/odbc/GNUmakefile
? src/interfaces/odbc/Makefile.global
? src/lextest/lex.yy.c
? src/lextest/lextest
? src/pl/plpgsql/src/Makefile
? src/pl/plpgsql/src/mklang.sql
? src/pl/plpgsql/src/pl_gram.c
? src/pl/plpgsql/src/pl.tab.h
? src/pl/plpgsql/src/pl_scan.c
? src/pl/tcl/mkMakefile.tcldefs.sh
? src/pl/tcl/Makefile.tcldefs
? src/test/regress/log
? src/test/regress/log2
Index: src/backend/catalog/heap.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.83
diff -c -r1.83 heap.c
*** src/backend/catalog/heap.c    1999/05/21 18:33:12    1.83
--- src/backend/catalog/heap.c    1999/05/22 04:05:41
***************
*** 1538,1563 ****

      if (type != atp->atttypid)
      {
!         /*
!          *    Though these types are binary compatible, bpchar has a fixed
!          *    length on the disk, requiring non-bpchar types to be padded
!          *    before storage in the default table.  bjm 1999/05/18
!          */
!         if (1==0 && atp->atttypid == BPCHAROID &&
!             (type == TEXTOID || type == BPCHAROID || type == UNKNOWNOID))
!         {
!
!             FuncCall   *n = makeNode(FuncCall);
!
!             n->funcname = typeidTypeName(atp->atttypid);
!             n->args = lcons((Node *)expr, NIL);
!             expr = transformExpr(NULL, (Node *) n, EXPR_COLUMN_FIRST);
!
!         }
!         else if (IS_BINARY_COMPATIBLE(type, atp->atttypid))
              ; /* use without change */
          else if (can_coerce_type(1, &(type), &(atp->atttypid)))
!             expr = coerce_type(NULL, (Node *)expr, type, atp->atttypid);
          else if (IsA(expr, Const))
          {
              if (*cast != 0)
--- 1538,1548 ----

      if (type != atp->atttypid)
      {
!         if (IS_BINARY_COMPATIBLE(type, atp->atttypid))
              ; /* use without change */
          else if (can_coerce_type(1, &(type), &(atp->atttypid)))
!             expr = coerce_type(NULL, (Node *)expr, type, atp->atttypid,
!                                                          atp->atttypmod);
          else if (IsA(expr, Const))
          {
              if (*cast != 0)
Index: src/backend/parser/parse_coerce.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/parser/parse_coerce.c,v
retrieving revision 2.14
diff -c -r2.14 parse_coerce.c
*** src/backend/parser/parse_coerce.c    1999/05/22 02:55:57    2.14
--- src/backend/parser/parse_coerce.c    1999/05/22 04:05:45
***************
*** 35,41 ****
   * Convert a function argument to a different type.
   */
  Node *
! coerce_type(ParseState *pstate, Node *node, Oid inputTypeId, Oid targetTypeId)
  {
      Node       *result = NULL;
      Oid            infunc;
--- 35,42 ----
   * Convert a function argument to a different type.
   */
  Node *
! coerce_type(ParseState *pstate, Node *node, Oid inputTypeId, Oid targetTypeId,
!         int32 atttypmod)
  {
      Node       *result = NULL;
      Oid            infunc;
***************
*** 82,92 ****
                  con->consttype = targetTypeId;
                  con->constlen = typeLen(typeidType(targetTypeId));

!                 /* use "-1" for varchar() type */
                  con->constvalue = (Datum) fmgr(infunc,
                                                 val,
                                                 typeidTypElem(targetTypeId),
!                                                -1);
                  con->constisnull = false;
                  con->constbyval = typeByVal(typeidType(targetTypeId));
                  con->constisset = false;
--- 83,98 ----
                  con->consttype = targetTypeId;
                  con->constlen = typeLen(typeidType(targetTypeId));

!                 /*
!                  *    Use "-1" for varchar() type.
!                  *    For char(), we need to pad out the type with the proper
!                  *    number of spaces.  This was a major problem for
!                  *  DEFAULT string constants to char() types.
!                  */
                  con->constvalue = (Datum) fmgr(infunc,
                                                 val,
                                                 typeidTypElem(targetTypeId),
!                                 (targetTypeId != BPCHAROID) ? -1 : atttypmod);
                  con->constisnull = false;
                  con->constbyval = typeByVal(typeidType(targetTypeId));
                  con->constisset = false;
***************
*** 100,106 ****
          result = node;

      return result;
! }    /* coerce_type() */


  /* can_coerce_type()
--- 106,112 ----
          result = node;

      return result;
! }


  /* can_coerce_type()
***************
*** 178,184 ****
      }

      return true;
! }    /* can_coerce_type() */


  /* TypeCategory()
--- 184,190 ----
      }

      return true;
! }


  /* TypeCategory()
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/parser/parse_expr.c,v
retrieving revision 1.46
diff -c -r1.46 parse_expr.c
*** src/backend/parser/parse_expr.c    1999/05/18 23:40:05    1.46
--- src/backend/parser/parse_expr.c    1999/05/22 04:05:45
***************
*** 417,423 ****
                      }
                      else if (can_coerce_type(1, &c->casetype, &ptype))
                      {
!                         c->defresult = coerce_type(pstate, c->defresult, c->casetype, ptype);
                          c->casetype = ptype;
                      }
                      else
--- 417,424 ----
                      }
                      else if (can_coerce_type(1, &c->casetype, &ptype))
                      {
!                         c->defresult = coerce_type(pstate, c->defresult,
!                                                     c->casetype, ptype, -1);
                          c->casetype = ptype;
                      }
                      else
***************
*** 439,445 ****
                      {
                          if (can_coerce_type(1, &wtype, &ptype))
                          {
!                             w->result = coerce_type(pstate, w->result, wtype, ptype);
                          }
                          else
                          {
--- 440,447 ----
                      {
                          if (can_coerce_type(1, &wtype, &ptype))
                          {
!                             w->result = coerce_type(pstate, w->result, wtype,
!                                                     ptype, -1);
                          }
                          else
                          {
Index: src/backend/parser/parse_func.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/parser/parse_func.c,v
retrieving revision 1.44
diff -c -r1.44 parse_func.c
*** src/backend/parser/parse_func.c    1999/05/17 17:03:33    1.44
--- src/backend/parser/parse_func.c    1999/05/22 04:05:53
***************
*** 352,358 ****
          }
          else
          {
-
              /*
               * Parsing aggregates.
               */
--- 352,357 ----
***************
*** 361,367 ****
              int                ncandidates;
              CandidateList    candidates;

-
              /*
               * the aggregate COUNT is a special case, ignore its base
               * type.  Treat it as zero
--- 360,365 ----
***************
*** 392,398 ****
                  type = agg_select_candidate(basetype, candidates);
                  if (OidIsValid(type))
                  {
!                     lfirst(fargs) = coerce_type(pstate