• Tom Lane's avatar
    Make INSERT-from-multiple-VALUES-rows handle targetlist indirection better. · a3c7a993
    Tom Lane authored
    Previously, if an INSERT with multiple rows of VALUES had indirection
    (array subscripting or field selection) in its target-columns list, the
    parser handled that by applying transformAssignedExpr() to each element
    of each VALUES row independently.  This led to having ArrayRef assignment
    nodes or FieldStore nodes in each row of the VALUES RTE.  That works for
    simple cases, but in bug #14265 Nuri Boardman points out that it fails
    if there are multiple assignments to elements/fields of the same target
    column.  For such cases to work, rewriteTargetListIU() has to nest the
    ArrayRefs or FieldStores together to produce a single expression to be
    assigned to the column.  But it failed to find them in the top-level
    targetlist and issued an error about "multiple assignments to same column".
    
    We could possibly fix this by teaching the rewriter to apply
    rewriteTargetListIU to each VALUES row separately, but that would be messy
    (it would change the output rowtype of the VALUES RTE, for example) and
    inefficient.  Instead, let's fix the parser so that the VALUES RTE outputs
    are just the user-specified values, cast to the right type if necessary,
    and then the ArrayRefs or FieldStores are applied in the top-level
    targetlist to Vars representing the RTE's outputs.  This is the same
    parsetree representation already used for similar cases with INSERT/SELECT
    syntax, so it allows simplifications in ruleutils.c, which no longer needs
    to treat INSERT-from-multiple-VALUES as its own special case.
    
    This implementation works by applying transformAssignedExpr to the VALUES
    entries as before, and then stripping off any ArrayRefs or FieldStores it
    adds.  With lots of VALUES rows it would be noticeably more efficient to
    not add those nodes in the first place.  But that's just an optimization
    not a bug fix, and there doesn't seem to be any good way to do it without
    significant refactoring.  (A non-invasive answer would be to apply
    transformAssignedExpr + stripping to just the first VALUES row, and then
    just forcibly cast remaining rows to the same data types exposed in the
    first row.  But this way would lead to different, not-INSERT-specific
    errors being reported in casting failure cases, so it doesn't seem very
    nice.)  So leave that for later; this patch at least isn't making the
    per-row parsing work worse, and it does make the finished parsetree
    smaller, saving rewriter and planner work.
    
    Catversion bump because stored rules containing such INSERTs would need
    to change.  Because of that, no back-patch, even though this is a very
    long-standing bug.
    
    Report: <20160727005725.7438.26021@wrigleys.postgresql.org>
    Discussion: <9578.1469645245@sss.pgh.pa.us>
    a3c7a993
ruleutils.c 281 KB