Refactor CopyFrom() in copyfrom.c.
authorFujii Masao <[email protected]>
Thu, 3 Oct 2024 06:59:16 +0000 (15:59 +0900)
committerFujii Masao <[email protected]>
Thu, 3 Oct 2024 06:59:16 +0000 (15:59 +0900)
This commit simplifies CopyFrom() by removing the unnecessary local variable
'skipped', which tracked the number of rows skipped due to on_error = 'ignore'.
That count is already handled by cstate->num_errors, so the 'skipped' variable
was redundant.

Additionally, the condition on_error != COPY_ON_ERROR_STOP is removed.
Since on_error == COPY_ON_ERROR_IGNORE is already checked, and on_error
only has two values (ignore and stop), the additional check was redundant
and made the logic harder to read. Seemingly this was introduced
in preparation for a future patch, but the current checks don’t offer
clear value and have been removed to improve readability.

Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com

src/backend/commands/copyfrom.c

index 47879994f7848ccc87af86f733191f94265c3c35..9139a407858d48b3562654c84463f5827af3b407 100644 (file)
@@ -657,7 +657,6 @@ CopyFrom(CopyFromState cstate)
    CopyMultiInsertInfo multiInsertInfo = {0};  /* pacify compiler */
    int64       processed = 0;
    int64       excluded = 0;
-   int64       skipped = 0;
    bool        has_before_insert_row_trig;
    bool        has_instead_insert_row_trig;
    bool        leafpart_use_multi_insert = false;
@@ -1004,26 +1003,22 @@ CopyFrom(CopyFromState cstate)
        if (!NextCopyFrom(cstate, econtext, myslot->tts_values, myslot->tts_isnull))
            break;
 
-       if (cstate->opts.on_error != COPY_ON_ERROR_STOP &&
+       if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE &&
            cstate->escontext->error_occurred)
        {
            /*
-            * Soft error occurred, skip this tuple and deal with error
-            * information according to ON_ERROR.
+            * Soft error occurred, skip this tuple and just make
+            * ErrorSaveContext ready for the next NextCopyFrom. Since we
+            * don't set details_wanted and error_data is not to be filled,
+            * just resetting error_occurred is enough.
             */
-           if (cstate->opts.on_error == COPY_ON_ERROR_IGNORE)
-
-               /*
-                * Just make ErrorSaveContext ready for the next NextCopyFrom.
-                * Since we don't set details_wanted and error_data is not to
-                * be filled, just resetting error_occurred is enough.
-                */
-               cstate->escontext->error_occurred = false;
+           cstate->escontext->error_occurred = false;
 
            /* Report that this tuple was skipped by the ON_ERROR clause */
            pgstat_progress_update_param(PROGRESS_COPY_TUPLES_SKIPPED,
-                                        ++skipped);
+                                        cstate->num_errors);
 
+           /* Repeat NextCopyFrom() until no soft error occurs */
            continue;
        }