Re: identity columns - Mailing list pgsql-hackers

On 2017-03-29 13:40:29 -0400, Peter Eisentraut wrote:
> On 3/24/17 05:29, Vitaly Burovoy wrote:
> > It would be great if "DROP IDENTITY IF EXISTS" is in the current patch
> > since we don't have any disagreements about "DROP IDENTITY" behavior
> > and easiness of implementation of "IF EXISTS" there.
> 
> Here is an updated patch that adds DROP IDENTITY IF EXISTS.
> 
> Unfortunately, implementing ADD IF NOT EXISTS is much harder, so I
> haven't done that here.
> 
> Additionally, this patch fixes a few minor issues that you had pointed
> out, and merges with the new expression evaluation system in the executor.

Your attachment has a mimetype of invalid/octet-stream - that's a first
for me ;)


Are you going to try to merge this soon, or are you pushing this to 11?
Feels a bit large for 10 at this point.


> +        case T_NextValueExpr:
> +            {
> +                NextValueExpr *nve = (NextValueExpr *) node;
> +
> +                scratch.opcode = EEOP_NEXTVALUEEXPR;
> +                scratch.d.nextvalueexpr.seqid = nve->seqid;
> +
> +                ExprEvalPushStep(state, &scratch);
> +                break;
> +            }

Hm - that's probably been answered somewhere, but why do we need a
special expression type for this?


>          default:
>              elog(ERROR, "unrecognized node type: %d",
>                   (int) nodeTag(node));
> diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
> index 4fbb5c1e74..5935b9ef75 100644
> --- a/src/backend/executor/execExprInterp.c
> +++ b/src/backend/executor/execExprInterp.c
> @@ -60,6 +60,7 @@
>  
>  #include "access/tuptoaster.h"
>  #include "catalog/pg_type.h"
> +#include "commands/sequence.h"
>  #include "executor/execExpr.h"
>  #include "executor/nodeSubplan.h"
>  #include "funcapi.h"
> @@ -337,6 +338,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
>          &&CASE_EEOP_NULLIF,
>          &&CASE_EEOP_SQLVALUEFUNCTION,
>          &&CASE_EEOP_CURRENTOFEXPR,
> +        &&CASE_EEOP_NEXTVALUEEXPR,
>          &&CASE_EEOP_ARRAYEXPR,
>          &&CASE_EEOP_ARRAYCOERCE,
>          &&CASE_EEOP_ROW,
> @@ -1228,6 +1230,14 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
>              EEO_NEXT();
>          }
>  
> +        EEO_CASE(EEOP_NEXTVALUEEXPR)
> +        {
> +            *op->resvalue = Int64GetDatum(nextval_internal(op->d.nextvalueexpr.seqid, false));
> +            *op->resnull = false;
> +
> +            EEO_NEXT();
> +        }

Is it guaranteed that the caller expects an int64?  I saw that
nextvalueexpr's have a typeid field.

Greetings,

Andres Freund